Page MenuHomeFreeBSD

bsdgrep(1): Revise test case for soon-to-be failure
ClosedPublic

Authored by kevans on May 2 2017, 4:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 5:00 AM
Unknown Object (File)
Thu, Nov 21, 8:52 AM
Unknown Object (File)
Sun, Nov 17, 9:26 PM
Unknown Object (File)
Oct 22 2024, 12:00 AM
Unknown Object (File)
Oct 17 2024, 10:39 AM
Unknown Object (File)
Oct 16 2024, 12:57 AM
Unknown Object (File)
Oct 16 2024, 12:57 AM
Unknown Object (File)
Oct 15 2024, 7:59 AM
Subscribers
None

Details

Reviewers
emaste
cem
ngie
Summary

D10315 is going to make egrep_empty_invalid an actually invalid regex,
to be consistent with the equivalent BRE "{" behavior, when using regex(3).

This test case is ultimately testing for the wrong thing- we're trying to detect
a clean exit, which can be pretty much any non-0 exit value depending on how the
installed grep interprets the expression. gnugrep interprets it as non-matching,
and in the future bsdgrep will interpret it is an error.

Ultimately, we don't care if grep exits with 1 or > 1 since interpretation is
implementation defined, so we use not-exit:0 instead to respect this.

Test Plan

Run kyua tests to ensure no regression, both with bsdgrep as /usr/bin/grep
and gnugrep as /usr/bin/grep

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9023
Build 9428: arc lint + arc unit

Event Timeline

usr.bin/grep/grep.c
742–745

This changes program flow if the qflag is present. Is that really correct? Probably qflag should cause us to exit(2) in place of errx(2, ...)?

usr.bin/grep/grep.c
742–745

I think it gets kind of sketchy here, depending on your interpretation of POSIX wording

If the -q option is specified, the exit status shall be zero if an input line is selected, even if an error was detected. Otherwise, default actions shall be performed.

My interpretation of this is that we *should* continue to try and match with what patterns we've accepted (if any) if the -q flag is specified. I may need to go back and add some more logic to make sure that this pattern that produced an error isn't used.

usr.bin/grep/grep.c
742–745

Further examination shows that trying to use such a pattern in regexec(3) will do the right thing and throw an error, introducing no further problems and leaving me no reason to introduce further logic to avoid using failed patterns.

If it will just error and not possibly match, what's the point of continuing on instead of exiting at the same place as !qflag?

In D10572#219108, @cem wrote:

If it will just error and not possibly match, what's the point of continuing on instead of exiting at the same place as !qflag?

Because we might have other patterns (multiple -e options) that *could* still be valid. Exiting shortly after could be desirable if we find that we actually don't have any valid patterns, though. Especially for large sets of input files.

My previous interpretation of POSIX was wrong; the term 'error' does not extend
to POSIX error and I was using a wrong test case against gnugrep. Back those bits out,
just revise the test to make sure we're exiting cleanly and not matching.

kevans retitled this revision from bsdgrep(1): Revise test case for soon-to-be failure, suppress errors properly to bsdgrep(1): Revise test case for soon-to-be failure.May 2 2017, 5:03 PM
kevans edited the summary of this revision. (Show Details)
In D10572#219129, @kevans91_ksu.edu wrote:

My previous interpretation of POSIX was wrong; the term 'error' does not extend
to POSIX error and I was using a wrong test case against gnugrep. Back those bits out,
just revise the test to make sure we're exiting cleanly and not matching.

What do you mean by POSIX error?

This revision is now accepted and ready to land.May 2 2017, 5:07 PM
contrib/netbsd-tests/usr.bin/grep/t_grep.sh
372–373

0 vs 1 has a distinct meaning with grep, and the above change loosens the exit code check. Per http://pubs.opengroup.org/onlinepubs/9699919799/utilities/grep.html :

EXIT STATUS

The following exit values shall be returned:

 0
One or more lines were selected.
 1
No lines were selected.
>1
An error occurred.

Which behavior are you expecting should be triggered: >1?

In D10572#219135, @ngie wrote:

What do you mean by POSIX error?

Uf, s/POSIX/regex/ -- the term 'error' doesn't seem to extend to regex errors after further examination, just other kinds of errors throughout execution.

contrib/netbsd-tests/usr.bin/grep/t_grep.sh
372–373

I'm expecting either: 1 or >1. For this particular expression, undefined behavior is abound and "{" could be either an error or some other valid interpretation, but in any case it should not die and at the very least not produce a match against /dev/null.

ngie added inline comments.
contrib/netbsd-tests/usr.bin/grep/t_grep.sh
372–373

Ok -- I agree.