Project

General

Profile

Actions

Bug #13027

closed

sed loses count of relative line offset

Added by Andy Fiddaman about 1 year ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
cmd - userland programs
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:

Description

As reported in https://illumos.topicbox.com/groups/discuss/T5a5499ff3291b3b2-Mcba7401e39f8b8a0dcb4b52e/a-bug-in-sed
sed has a bug handling relative offsets. It turns out that there are two bugs here:

The first is that sed loses count of its place if the relative line count takes it to the penultimate line in the file:

build% cat l
a
b
c
d
a
b
c
d
build% sed -n '4,+2p' l
d
a
b
build% sed -n '5,+2p' l
a
b
c
d

and the second is that with a pattern and relative line count, the pattern is not re-matched against the correct line (the one following the matched range):

build% sed -n '/a/,+3p' l
a
b
c
d

This behaviour is a result of a couple of issues in the current code.
There's no explicit test for reaching the last line in a relative range block. This means that the range is only terminated by the later condition that checks if the current line is now outside of the block. However, this check is preceded by a MATCH on the second address. Since the MATCH macro does not handle the relative range type it defaults to checking if the current line is the last one. That means that if the line following the desired range is the last, then it will match even though it is outside of the range.

Actions #1

Updated by Electric Monk about 1 year ago

  • Gerrit CR set to 836
Actions #2

Updated by Andy Fiddaman about 1 year ago

I ran the new tests with the original sed and got three failures:

% ./sed_addr.ksh
[PASS] sed 5,7p
[PASS] sed 5,4p
[PASS] sed /a/,4p
[PASS] sed 0,/b/p
[PASS] sed $p
[PASS] sed 4,/a/p
[PASS] sed /d/,/g/p
[PASS] sed 3,+0p
[PASS] sed 3,+1p
[PASS] sed 5,+3p
[FAIL] sed 6,+3p
--- /tmp/tmp_28.43d     Fri Aug  7 22:39:01 2020
+++ /tmp/tmp_16.e5k     Fri Aug  7 22:39:01 2020
@@ -2,3 +2,4 @@
 g
 h
 a
+j
[PASS] sed 7,+3p
[PASS] sed 8,+3p
[PASS] sed /a/,+1p
[FAIL] sed /a/,+8p
--- /tmp/tmp_1l.6db     Fri Aug  7 22:39:01 2020
+++ /tmp/tmp_16.e5k     Fri Aug  7 22:39:01 2020
@@ -7,3 +7,4 @@
 g
 h
 a
+j
[PASS] sed /a/,+9p
[PASS] sed 4,7!p
[FAIL] sed 6,+3!p
--- /tmp/tmp_29.k23     Fri Aug  7 22:39:01 2020
+++ /tmp/tmp_16.e5k     Fri Aug  7 22:39:01 2020
@@ -3,4 +3,3 @@
 c
 d
 e
-j
[PASS] sed 7,+3!p
[PASS] sed 8,+3!p

With the updated version (see associated review), all of these tests pass:

% PATH=../../../../cmd/sed:$PATH ./sed_addr.ksh
[PASS] sed 5,7p
[PASS] sed 5,4p
[PASS] sed /a/,4p
[PASS] sed 0,/b/p
[PASS] sed $p
[PASS] sed 4,/a/p
[PASS] sed /d/,/g/p
[PASS] sed 3,+0p
[PASS] sed 3,+1p
[PASS] sed 5,+3p
[PASS] sed 6,+3p
[PASS] sed 7,+3p
[PASS] sed 8,+3p
[PASS] sed /a/,+1p
[PASS] sed /a/,+8p
[PASS] sed /a/,+9p
[PASS] sed 4,7!p
[PASS] sed 6,+3!p
[PASS] sed 7,+3!p
[PASS] sed 8,+3!p
Actions #3

Updated by Andy Fiddaman about 1 year ago

And for completness, with GNU sed, the results are the same barring the test for a range starting with line 0, which is a result of a GNU extension here.

% cp =gsed /tmp/sed
% PATH=/tmp:$PATH ./sed_addr.ksh
[PASS] sed 5,7p
[PASS] sed 5,4p
[PASS] sed /a/,4p
[FAIL] sed 0,/b/p
--- /tmp/tmp_33.fhe     Fri Aug  7 22:42:52 2020
+++ /tmp/tmp_3u.vvf     Fri Aug  7 22:42:52 2020
@@ -1,0 +1,2 @@
+a
+b
[PASS] sed $p
[PASS] sed 4,/a/p
[PASS] sed /d/,/g/p
[PASS] sed 3,+0p
[PASS] sed 3,+1p
[PASS] sed 5,+3p
[PASS] sed 6,+3p
[PASS] sed 7,+3p
[PASS] sed 8,+3p
[PASS] sed /a/,+1p
[PASS] sed /a/,+8p
[PASS] sed /a/,+9p
[PASS] sed 4,7!p
[PASS] sed 6,+3!p
[PASS] sed 7,+3!p
[PASS] sed 8,+3!p
Actions #4

Updated by Andy Fiddaman about 1 year ago

  • Description updated (diff)
Actions #5

Updated by Electric Monk about 1 year ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 4e81fcfe0bcce62e29d80c1e391d9a241c0ee3b1

commit  4e81fcfe0bcce62e29d80c1e391d9a241c0ee3b1
Author: Andy Fiddaman <omnios@citrus-it.co.uk>
Date:   2020-08-14T20:48:48.000Z

    13027 sed loses count of relative line offset
    13028 system/test/utiltest package is missing licence metadata
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF