Bug 192108 - [patch] sed(1) bug in "addr1,+N" ranges
Summary: [patch] sed(1) bug in "addr1,+N" ranges
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 9.2-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Jeremie Le Hen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-25 09:48 UTC by Rudolf Čejka
Modified: 2015-01-10 10:16 UTC (History)
2 users (show)

See Also:

Attachments
Proposed patch for review (857 bytes, patch)
2014-07-25 09:48 UTC, Rudolf Čejka 		no flags Details | Diff
Fix sed relative address (2.06 KB, patch)
2014-07-26 08:55 UTC, Jeremie Le Hen 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rudolf Čejka 2014-07-25 09:48:39 UTC 
Created attachment 144962 [details]
Proposed patch for review

There is a bug in "addr1,+N" ranges in the case of last input line, let's try:

# seq 1 4 | sed -ne '1,+0p'
1
# seq 1 4 | sed -ne '1,+1p'
1
2
# seq 1 4 | sed -ne '1,+2p'
1
2
3
4                               <--- This line should not be printed!!!
# seq 1 4 | sed -ne '1,+3p'
1
2
3
4

The reason is that in case of AT_RELLINE, lastline()'s TRUE in MATCH() is taken before the test for relative range next (process.c:291-297):

                        ...
                        if (MATCH(cp->a2)) {
                                cp->startline = 0;
                                lastaddr = 1;
                                r = 1;
                        } else if (linenum - cp->startline <= cp->a2->u.l)
                                r = 1;
                        else if ((cp->a2->type == AT_LINE &&
                        ...
Comment 1 Jeremie Le Hen freebsd_committer freebsd_triage 2014-07-25 09:56:20 UTC 
Thanks for reporting.

I'll take care of it this weekend.
Comment 2 Jeremie Le Hen freebsd_committer freebsd_triage 2014-07-26 08:55:27 UTC 
Created attachment 144995 [details]
Fix sed relative address

Here is the patch I propose.  Would you mind testing it please?  I did and it seems to work for me, but you cover some cases I haven't thought of.

Additionally the patch untangles a bit the logic and makes the code easier to grasp, at the expense of being a bit longer
Comment 3 Rudolf Čejka 2014-07-30 09:22:52 UTC 
Hello, your patch seems good to me too, so please commit it ;o)

Thank you very much.
Comment 4 commit-hook freebsd_committer freebsd_triage 2014-07-30 14:47:03 UTC 
A commit references this bug:

Author: jlh
Date: Wed Jul 30 14:46:40 UTC 2014
New revision: 269302
URL: http://svnweb.freebsd.org/changeset/base/269302

Log:
  Fix relative numerical addressing (addr,+N).

  As a bonus the patch untangles a bit the logic and makes the code
  easier to grasp.

  PR:		192108
  MFC after:	1 week

Changes:
  head/usr.bin/sed/process.c
Comment 5 commit-hook freebsd_committer freebsd_triage 2014-08-11 20:39:49 UTC 
A commit references this bug:

Author: jlh
Date: Mon Aug 11 20:38:52 UTC 2014
New revision: 269837
URL: http://svnweb.freebsd.org/changeset/base/269837

Log:
  MFC r269302:

    Fix relative numerical addressing (addr,+N).

    As a bonus the patch untangles a bit the logic and makes the code
    easier to grasp.

    PR:           192108

Changes:
_U  stable/10/
  stable/10/usr.bin/sed/process.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2014-08-11 21:38:59 UTC 
A commit references this bug:

Author: jlh
Date: Mon Aug 11 21:38:42 UTC 2014
New revision: 269841
URL: http://svnweb.freebsd.org/changeset/base/269841

Log:
  MFC r269302:

    Fix relative numerical addressing (addr,+N).

    As a bonus the patch untangles a bit the logic and makes the code
    easier to grasp.

    PR:           192108

Changes:
_U  stable/9/usr.bin/sed/
  stable/9/usr.bin/sed/process.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2015-01-10 10:16:40 UTC 
A commit references this bug:

Author: jlh
Date: Sat Jan 10 10:16:23 UTC 2015
New revision: 276909
URL: https://svnweb.freebsd.org/changeset/base/276909

Log:
  Add a regression test for PR 192108.

  I won't go through the hassle of MFCing it since I expect all changes to go
  first through HEAD anyway.

  PR:		192108

Changes:
  head/usr.bin/sed/tests/multi_test.sh
  head/usr.bin/sed/tests/regress.multitest.out/2.23