Subject: Re: bug#19899: deleting lines of a file with sed - unexpected behaviour

From: Norihiro Tanaka <[email-masked]>
Date: Fri, 20 Feb 2015 02:13:58 +0900

On Wed, 18 Feb 2015 19:56:58 -0500
Ethan Kaufman <[email-masked]> wrote:

> To whom it may concern,
> 
> I noticed something odd while fooling around with sed.  If you try to
> remove multiple line intervals (by number) from a file, but any interval
> specified later in the list is a subset of an interval earlier in the list,
> then an additional single line is removed after the specified (larger)
> interval.
> 
> seq 10 > foo.txt
> 
> sed '2,7d;3,6d' foo.txt
> 1
> 9
> 10
> 
> Expected output is:
> 
> 1
> 8
> 9
> 10
> 
> Additional tests:
> 
> For each additional redundant interval, another line is removed:
> sed '2,7d;3,6d;4,5d' foo.txt
> 1
> 10
> 
> Reversing the order of the intervals produces the expected result!
> sed '3,6d;2,7d' foo.txt
> 1
> 8
> 9
> 10
> 
> Specifying the intervals with '-e' produces the same result:
> sed -e '2,7d' -e '3,6d' foo.txt
> 1
> 9
> 10
> 
> Using different interval syntax has mixed results:
> sed -e '/2/,/7/d' -e '/3/,/6/d' foo.txt
> 1
> 8
> 9
> 10
> sed -e '2,7d' -e '/3/,/6/d' foo.txt
> 1
> 8
> 9
> 10
> sed -e '/2/,/7/d' -e '3,6d' foo.txt
> 1
> 9
> 10
> 
> Trailing list must be a subset for the additional line to be removed:
> 
> sed '2,5d;1,5d'
> 1
> 8
> 9
> 10
> sed '2,5d;2,6d'
> 1
> 8
> 9
> 10
> sed '2,5d;2,5d'
> 1
> 9
> 10
> 
> Versions:
> 
> Breakage appears to have occurred in the 4.1 release.  See the expected
> output for all cases in GNU sed 3.02 and 4.09 (as well as BSD sed (Mac OS X
> 10.2 Yosemite and /bin/sed on Solaris), but not in 4.15 and 4.21.
> 
> This issue and above information has been discussed on stack overflow:
> stackoverflow.com/questions/28595574/deleting-lines-of-a-file-with-sed-unexpected-behaviour
> 
> Cheers,
> Ethan

Hi Ethan,

I see that the behavior is a bug.  I tested following sed-4.2.2, old GNU
sed, sed on Solaris 11 and sed on HP-UX, and sed-4.2.2 only does not
return '8'.

  $ sed '2,7d;3,6d' foo.txt

When line number addresses are overwrapped in two editing commands,
sed always executes for at least one line with the second edition
command, even if out of between two addresses.  e.g. for '2,7d;3,6d',
sed executes for line 8 which should not be executed for.

Thanks,
Norihiro
From 3d65ca4dbf3d74d81354861e3a7130267eac3c2b Mon Sep 17 00:00:00 2001
From: Norihiro Tanaka <[email-masked]>
Date: Fri, 20 Feb 2015 01:54:35 +0900
Subject: [PATCH] sed: fix mishandle with overwrapped line number addresses

When line number addresses are overwrapped in two editing commands,
sed always executes for at least one line with the second edition
command, even if out of between two addresses.  e.g. for '2,7d;3,6d',
sed executes for line 8 which should not be executed for.

* sed/execute.c (match_an_address_p): Add movement for line number
addresses.
(match_address_p): Change as not executed if out of between two addresses.
* testsuite/overwrap.good, : New file, to test for this.
* testsuite/overwrap.inp: New file, to test for this.
* testsuite/overwrap.sed: New file, to test for this.
* testsuite/Makefile.am (SEDTESTS, EXTRA_DIST): Add a rule for the new test.
* testsuite/Makefile.tests: Add a rule for the new test.
* NEWS (Bug fixes): Mention it.
---
 NEWS                     |  6 ++++++
 sed/execute.c            | 16 ++++++++--------
 testsuite/Makefile.am    |  3 ++-
 testsuite/Makefile.tests |  2 +-
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index b713b1d..26f70d1 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,12 @@ GNU sed NEWS                                    -*- outline -*-
   is what stopped working. [bug introduced some time after sed-3.02 and
   prior to the first sed-4* test release]
 
+  When line number addresses are overwrapped in two editing commands,
+  sed always executes for at least one line with the second edition
+  command, even if out of between two addresses.  e.g. for '2,7d;3,6d',
+  sed executes for line 8 which should not be executed for. [bug
+  introduced some time after sed-4.09 and prior to release in sed-4.1]
+
 ** Feature removal
 
   The "L" command (format a paragraph like the fmt(1) command would)
diff --git a/sed/execute.c b/sed/execute.c
index 8510a15..547d3ef 100644
--- a/sed/execute.c
+++ b/sed/execute.c
@@ -798,8 +798,10 @@ match_an_address_p(struct addr *addr, struct input *input)
     case ADDR_IS_LAST:
       return test_eof(input);
 
-      /* ADDR_IS_NUM is handled in match_address_p.  */
     case ADDR_IS_NUM:
+      /* reminder: these are only meaningful for a1 addresses */
+      return (addr->addr_number == input->line_number);
+
     default:
       panic("INTERNAL ERROR: bad address type");
     }
@@ -816,23 +818,20 @@ match_address_p(struct sed_cmd *cmd, struct input *input)
 
   if (cmd->range_state != RANGE_ACTIVE)
     {
+      if (!cmd->a2)
+        return match_an_address_p(cmd->a1, input);
+
       /* Find if we are going to activate a range.  Handle ADDR_IS_NUM
          specially: it represent an "absolute" state, it should not
          be computed like regexes.  */
       if (cmd->a1->addr_type == ADDR_IS_NUM)
         {
-          if (!cmd->a2)
-            return (input->line_number == cmd->a1->addr_number);
-
           if (cmd->range_state == RANGE_CLOSED
               || input->line_number < cmd->a1->addr_number)
             return false;
         }
       else
         {
-          if (!cmd->a2)
-            return match_an_address_p(cmd->a1, input);
-
           if (!match_an_address_p(cmd->a1, input))
             return false;
         }
@@ -848,7 +847,8 @@ match_address_p(struct sed_cmd *cmd, struct input *input)
           /* Same handling as below, but always include at least one line.  */
           if (input->line_number >= cmd->a2->addr_number)
             cmd->range_state = RANGE_CLOSED;
-          return true;
+          return (input->line_number <= cmd->a2->addr_number
+                  || match_an_address_p(cmd->a1, input));
         case ADDR_IS_STEP:
           cmd->a2->addr_number = input->line_number + cmd->a2->addr_step;
           return true;
diff --git a/testsuite/Makefile.am b/testsuite/Makefile.am
index 060033b..2895196 100644
--- a/testsuite/Makefile.am
+++ b/testsuite/Makefile.am
@@ -26,7 +26,7 @@ SEDTESTS += \
 	insens subwrite writeout readin insert utf8-1 utf8-2 utf8-3 utf8-4 \
 	badenc inplace-hold brackets amp-escape \
 	help version file quiet \
-	factor binary3 binary2 binary dc
+	factor binary3 binary2 binary dc overwrap
 
 TESTS_ENVIRONMENT = MAKE="$(MAKE)" VERSION="$(VERSION)" \
 	$(LOCALCHARSET_TESTS_ENVIRONMENT)
@@ -77,6 +77,7 @@ EXTRA_DIST = \
 	numsub3.good numsub3.inp numsub3.sed \
 	numsub4.good numsub4.inp numsub4.sed \
 	numsub5.good numsub5.inp numsub5.sed \
+	overwrap.good overwrap.inp overwrap.sed \
 	readin.good readin.inp readin.sed \
 	recall.good recall.inp recall.sed \
 	recall2.good recall2.inp recall2.sed \
diff --git a/testsuite/Makefile.tests b/testsuite/Makefile.tests
index 7f50f50..80d5fe6 100644
--- a/testsuite/Makefile.tests
+++ b/testsuite/Makefile.tests
@@ -21,7 +21,7 @@ SKIP = :>$@.skip; exit 77
 enable sep inclib 8bit 8to7 newjis xabcx dollar noeol bkslashes \
 numsub head madding mac-mf empty xbxcx xbxcx3 recall recall2 xemacs \
 appquit fasts uniq manis linecnt khadafy allsub flipcase space modulo \
-y-bracket y-newline y-zero insert brackets amp-escape::
+y-bracket y-newline y-zero insert brackets amp-escape overwrap::
 	$(SEDENV) $(SED) -f $(srcdir)/$@.sed \
 		< $(srcdir)/$@.inp | $(TR) -d \\r > $@.out
 	$(CMP) $(srcdir)/$@.good $@.out
-- 
2.2.0