Details

Reviewers

emaste
cem
ngie

Commits

rS317703: bsdgrep: fix -w flag matching with an empty pattern

Summary

-w flag matching with an empty pattern was generally 'broken', allowing
matches to occur on any line whether or not it actually matches -w criteria.

This one required a good amount of refactoring to address. Basically, procline()
gets altered to *only* process the line and return whether it was a match or not,
necessary to be able to short-circuit the whole function in case of this matchall
flag. We move out the -m flag handling at the same time because it suffers from
the same fate as context handling if we bypass any actual pattern matching.

The matching context (matches, mostly) didn't previously exist outside of procline(),
so we go ahead and create context object for file processing bits to pass around.
grep_printline() was created due to this, for the scenarios where the matches don't
actually matter and we just want to print a line or two, a la flushing the context
queue and no -o or --color specified.

Damage from this broken behavior would have been mitigated by the fact that the
odds are really low that one would depend on empty patterns with the -w flag.

This was spotted while checking PR 105221 for problems that this may cause in
bsdgrep(1), but 105221 is *not* a report of this behavior.

Test Plan

Ensure no regressions in Kyua tests

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 8803
Build 9172: arc lint + arc unit

Event Timeline

kevans created this revision.Apr 20 2017, 2:48 AM

Herald added a reviewer: ngie. · View Herald TranscriptApr 20 2017, 2:48 AM

Apologies, my working directory changes did not make it into a commit for
the initial submission of this. procline()'s return value was supposed to be changed
to instead use the normal pattern (return of 0 is good/match, 1 is no good/match)

Fix merge conflicts as a result of rS317254

Harbormaster completed remote builds in B8825: Diff 27607.Apr 21 2017, 3:00 PM

Fix (?) ordering

Rewrite/fix the context handling; this was missed because I forgot to change the
atf_expect_fail with rS317051, so I completely broke context handling but missed
it because the tests failed as expected. This version has some added benefits:

The queue handling for -B was previously done whether we actually cared about context or not; this is no longer the case
The flow for printing output is a lot more linear and readable, IMO. It's pretty clear now where the matched line is printed and where the context is printed.

Harbormaster completed remote builds in B8827: Diff 27609.Apr 21 2017, 6:18 PM

This should be the last tweak to this one:

Simplify even further; ctxover isn't necessary, we can just inspect linesqueued

Sorry, I lied; simplify context separator bits a little bit further

Harbormaster completed remote builds in B8830: Diff 27612.Apr 21 2017, 7:26 PM

More simplification...we can always get better, right?

Harbormaster completed remote builds in B8833: Diff 27616.Apr 21 2017, 7:55 PM

Fix one last context nit that we weren't testing for and I stumbled upon; adjacent non-overlapping contexts were still broken

Harbormaster completed remote builds in B8844: Diff 27628.Apr 22 2017, 3:49 AM

Apologies for the phab spam; I think that's it. The last one was only found after throwing stuff at the wall for the better part of two hours, so I think it's safe to say this has settled down.

Further amend tests for two other cases; properly measure gap between context/match by whether we're rotating lines out of the queue or not doing -B matches