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

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 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 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 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 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 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 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