Bug 253350

Summary: Grep regression with --after-context (-A) and --max-count (-m) switches
Product: Base System Reporter: David Schlachter <fbsd-bugzilla>
Component: binAssignee: Kyle Evans <kevans>
Status: Closed FIXED    
Severity: Affects Only Me CC: bugs, kevans, lwhsu
Priority: --- Flags: kevans: mfc-stable13+
kevans: mfc-stable12+
kevans: mfc-stable11-
Version: 13.0-STABLE   
Hardware: Any   
OS: Any   

Description David Schlachter 2021-02-08 16:13:58 UTC
Example file:

$ cat test-grep.txt 
foo
bar
foo
bar
foo
bar

grep behavior in 12.2-RELEASE:

$ grep -A1 -m1 foo test-grep.txt 
foo
bar

grep behavior in 13.0-BETA1:

$ grep -A1 -m1 foo test-grep.txt
foo
Comment 1 Kyle Evans freebsd_committer freebsd_triage 2021-02-08 16:56:43 UTC
Thanks for the report!

This diff will fix it: https://people.freebsd.org/~kevans/grep-m.diff

We previously took the mcount hit to bail out immediately, this change ensures that we continue processing lines as non-matches until the -A tail runs out. i.e., changing your second line from bar to foo and re-running the same scenario should output the first foo as a match and the second as a non-match with appropriate delimiter with -n specified.

I will clean this up and add a test to commit it a little later today.
Comment 2 commit-hook freebsd_committer freebsd_triage 2021-02-08 18:41:57 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3e2d96ac974db823255a6f40b90eeffa6e38d022

commit 3e2d96ac974db823255a6f40b90eeffa6e38d022
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2021-02-08 18:31:17 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2021-02-08 18:41:22 +0000

    grep: fix -A handling in conjunction with -m match limitation

    The basic issue here is that grep, when given -m 1, would stop all
    line processing once it hit the match count and exit immediately.  The
    problem with exiting immediately is that -A processing only happens when
    subsequent lines are processed and do not match.

    The fix here is relatively easy; when bsdgrep matches a line, it resets
    the 'tail' of the matching context to the value supplied to -A and
    dumps anything that's been queued up for -B. After the current line has
    been printed and tail is reset, we check our mcount and do what's
    needed. Therefore, at the time that we decide we're doing nothing, we
    know that 'tail' of the context is correct and we can simply continue
    on if there's still more to pick up.

    With this change, we still bail out immediately if there's been no -A
    flag. If -A was supplied, we signal that we should continue on. However,
    subsequent lines will not even bothere to try and process the line.  We
    have reached the match count, so even if the next line would match then
    we must process it if it hadn't. Thus, the loop in procfile() can
    short-circuit and just process the line as a non-match until
    procmatches() indicates that it's safe to stop.

    A test has been added to reflect both that we should be picking up the
    next line and that the next line should be considered a non-match even
    if it should have been.

    PR:             253350
    MFC-after:      3 days

 contrib/netbsd-tests/usr.bin/grep/t_grep.sh | 17 +++++++++++++++++
 usr.bin/grep/util.c                         | 21 ++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-02-11 02:50:47 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=08f25b50dbd332e5c5c9380fd90c516e9af1ab36

commit 08f25b50dbd332e5c5c9380fd90c516e9af1ab36
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2021-02-08 18:31:17 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2021-02-11 02:49:59 +0000

    grep: fix -A handling in conjunction with -m match limitation

    The basic issue here is that grep, when given -m 1, would stop all
    line processing once it hit the match count and exit immediately.  The
    problem with exiting immediately is that -A processing only happens when
    subsequent lines are processed and do not match.

    The fix here is relatively easy; when bsdgrep matches a line, it resets
    the 'tail' of the matching context to the value supplied to -A and
    dumps anything that's been queued up for -B. After the current line has
    been printed and tail is reset, we check our mcount and do what's
    needed. Therefore, at the time that we decide we're doing nothing, we
    know that 'tail' of the context is correct and we can simply continue
    on if there's still more to pick up.

    With this change, we still bail out immediately if there's been no -A
    flag. If -A was supplied, we signal that we should continue on. However,
    subsequent lines will not even bothere to try and process the line.  We
    have reached the match count, so even if the next line would match then
    we must process it if it hadn't. Thus, the loop in procfile() can
    short-circuit and just process the line as a non-match until
    procmatches() indicates that it's safe to stop.

    A test has been added to reflect both that we should be picking up the
    next line and that the next line should be considered a non-match even
    if it should have been.

    PR:             253350
    (cherry picked from commit 3e2d96ac974db823255a6f40b90eeffa6e38d022)

 contrib/netbsd-tests/usr.bin/grep/t_grep.sh | 17 +++++++++++++++++
 usr.bin/grep/util.c                         | 21 ++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2021-02-11 15:12:51 UTC
A commit in branch releng/13.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=dc57f212526d25e8ef036e768b816a7fdae5707b

commit dc57f212526d25e8ef036e768b816a7fdae5707b
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2021-02-11 15:11:24 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2021-02-11 15:12:17 +0000

    grep: fix -A handling in conjunction with -m match limitation

    The basic issue here is that grep, when given -m 1, would stop all
    line processing once it hit the match count and exit immediately.  The
    problem with exiting immediately is that -A processing only happens when
    subsequent lines are processed and do not match.

    The fix here is relatively easy; when bsdgrep matches a line, it resets
    the 'tail' of the matching context to the value supplied to -A and
    dumps anything that's been queued up for -B. After the current line has
    been printed and tail is reset, we check our mcount and do what's
    needed. Therefore, at the time that we decide we're doing nothing, we
    know that 'tail' of the context is correct and we can simply continue
    on if there's still more to pick up.

    With this change, we still bail out immediately if there's been no -A
    flag. If -A was supplied, we signal that we should continue on. However,
    subsequent lines will not even bothere to try and process the line.  We
    have reached the match count, so even if the next line would match then
    we must process it if it hadn't. Thus, the loop in procfile() can
    short-circuit and just process the line as a non-match until
    procmatches() indicates that it's safe to stop.

    A test has been added to reflect both that we should be picking up the
    next line and that the next line should be considered a non-match even
    if it should have been.

    PR:             253350
    Approved by:    re (gjb)

    (cherry picked from commit 3e2d96ac974db823255a6f40b90eeffa6e38d022)
    (cherry picked from commit 08f25b50dbd332e5c5c9380fd90c516e9af1ab36)

 contrib/netbsd-tests/usr.bin/grep/t_grep.sh | 17 +++++++++++++++++
 usr.bin/grep/util.c                         | 21 ++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-02-11 15:21:55 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ad488c9793fd5040f8267620f370f319df31af2d

commit ad488c9793fd5040f8267620f370f319df31af2d
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2021-02-08 18:31:17 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2021-02-11 15:20:26 +0000

    grep: fix -A handling in conjunction with -m match limitation

    The basic issue here is that grep, when given -m 1, would stop all
    line processing once it hit the match count and exit immediately.  The
    problem with exiting immediately is that -A processing only happens when
    subsequent lines are processed and do not match.

    The fix here is relatively easy; when bsdgrep matches a line, it resets
    the 'tail' of the matching context to the value supplied to -A and
    dumps anything that's been queued up for -B. After the current line has
    been printed and tail is reset, we check our mcount and do what's
    needed. Therefore, at the time that we decide we're doing nothing, we
    know that 'tail' of the context is correct and we can simply continue
    on if there's still more to pick up.

    With this change, we still bail out immediately if there's been no -A
    flag. If -A was supplied, we signal that we should continue on. However,
    subsequent lines will not even bothere to try and process the line.  We
    have reached the match count, so even if the next line would match then
    we must process it if it hadn't. Thus, the loop in procfile() can
    short-circuit and just process the line as a non-match until
    procmatches() indicates that it's safe to stop.

    A test has been added to reflect both that we should be picking up the
    next line and that the next line should be considered a non-match even
    if it should have been.

    PR:             253350

    (cherry picked from commit 3e2d96ac974db823255a6f40b90eeffa6e38d022)

 contrib/netbsd-tests/usr.bin/grep/t_grep.sh | 17 +++++++++++++++++
 usr.bin/grep/util.c                         | 21 ++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)
Comment 6 Kyle Evans freebsd_committer freebsd_triage 2021-02-11 15:26:46 UTC
This should be in -BETA2. Thanks for the report!