bsdgrep(1): Don't output matches with -c, -l flag, and -L flags

[kevans]

Summary

Refactoring done in rS317703 broke -c, -l, and -L flags implying suppression of
match printing. Fortunately, this is just a matter of not doing any printing
of the resulting matches and context printing was not broken in this refactoring.

Add some regression tests since this area may still see further refactoring, include
different context flags as well even though they were not broken in this case.

Reported by: markj@
PR: 219077

Test Plan

Ensure no regressions in Kyua tests, test build libsysdecode with bsdgrep
installed as /usr/bin/grep as in the reported PR.

[kevans]

• Remove changes snuck into the revision that did not appear in the diff I did beforehand...

[cem]

LGTM minus what I guess is an accidental change to bnxt.

sys/dev/bnxt/bnxt.h

81 ↗

(On Diff #28047)

Unrelated change?

[kevans]

In D10607#220010, @cem wrote:

LGTM minus what I guess is an accidental change to bnxt.

Yeah, I always diff it against 'master' (git) before submission, but this didn't show up so I'm guessing I should be diffing against...something else. It's slightly confusing, though, because this is the only machine that I actually manipulate this repo and its remotes on.

[cem]

In D10607#220014, @kevans91_ksu.edu wrote:

Yeah, I always diff it against 'master' (git) before submission, but this didn't show up so I'm guessing I should be diffing against...something else. It's slightly confusing, though, because this is the only machine that I actually manipulate this repo and its remotes on.

How do you create the review? I tend to use arc diff --allow-untracked --create HEAD^, so I'd guess I'd diff HEAD against HEAD^. If you're creating a review against master instead of HEAD^, I think diffing against it is correct. I don't know :-).

[kevans]

• Further inspection showed problem a little more widespread, having broken -c, -l, and -L. Add appropriate tests for all of them, making sure that context still doesn't break them for some reason (except for -L, since it's explicitly non-match)
• Use a 'printmatch' bool, similar to 'doctx,' so that we're not checking all of the flags every iteration in some cases and to be consistent with context printing

[kevans]

• Go final step further, simplify doctx checking to whether we're printing matches and context flags are used

[ngie]

usr.bin/grep/util.c

244 ↗

(On Diff #28049)

Shouldn't pc.binary be (pc.binary && binbehave != BINFILE_TEXT) (is a binary file and -a or --binary-files=text is specified)? If so, this probably deserves its own testcase.

[kevans]

I actually don't bother specifying diff target, just arc diff --create and let it default. I should probably be more specific to it so that I don't get surprises like this. =)

[ngie]

usr.bin/grep/util.c

244 ↗

(On Diff #28049)

It looks like you incorporated some of my recommendations while I was typing this up, but I'm still curious about BINFILE_BIN vs BINFILE_TEXT.

Also, if you could add some more testcases for -I/--binary-files behavior w.r.t. these flags, that would be awesome :).

[kevans]

usr.bin/grep/util.c

244 ↗

(On Diff #28049)

Indeed- I tossed around != BINFILE_TEXT and == BINFILE_BIN, and determined that == BINFILE_BIN might be a little bit better. BINFILE_SKIP and BINFILE_TEXT are both harmless since the former breaks out before any printing and the latter is, well, what it is. I think that we're less likely to add another BINFILE_* value that gets treated as BINFILE_BIN than we are to add another BINFILE_* flag that allows safe behavior here.

re: binary file test cases, I wish to do that in a separate patch if that's alright. Our test coverage for all of the binary file behaviors is currently at 0%, aside from context2 that forces what would otherwise be detected as a 'binary' file to be detected as text through -z/--null-data. I don't entirely count that as testing binary behavior, though, so 0% is a final number.

I think it makes sense to create a single binary test that checks -a, -I, -U, and --binary-files= with the couple of different combinations that could feasibly break things.

[ngie]

usr.bin/grep/util.c

244 ↗

(On Diff #28049)

Sounds good to me -- thanks for the reply!