bsdgrep(1): Fix --mmap behavior

[kevans]

Summary

rS313948 partially fixed --mmap behavior but took an odd and incomplete approach. This commit
generally reverts it and does it the more correct way- by just consuming the rest of the buffer
and moving on.

The previous approach was wrong because that's just now how --mmap operates. The part of the loop
now behind a filebehave != FILE_MMAP is only to see if we hit an EOL/EOF in part of the file we had
not yet read in. There's no point in doing this when for --mmap because we already have the entire buffer
mapped, so if we hit this loop then we're really looking at EOF. The previous fix was resulting in getting
hit by a (SIG)BUS in certain scenarios, such as reported in PR 219402, because we were copying past the end of
the lnbuf for reasons that don't make sense.

PR: 219402

Test Plan

Run kyua tests to ensure no regressions, as well as test cases presented in PR 219402 and PR 165471 to
make sure --mmap still works.

[cem]

usr.bin/grep/file.c

222 ↗

(On Diff #28569)

If you did:

if (filebehave == FILE_MMAP) {
    bufrem -= len;
    continue;
}

You could save indenting the rest of the longer code path. I also like that the shorter side of the branch is closer to the condition.

This is all just my personal preference and not mandatory at all.

[emaste]

usr.bin/grep/file.c

222 ↗

(On Diff #28569)

I also prefer early returns, use of continue, etc. to reduce indentation.

[kevans]

• Re-work based on comments by cem@, emaste@; revise comments to read better with the flow while expressing the same intent

[emaste]

Would be useful to have a test case for this

[kevans]

In D10820#224321, @emaste wrote:

Would be useful to have a test case for this

Yeah, so, I'm slowly throwing one together -- we need to cover normal scenarios as well as this one, but I've been trying to narrow this down to a simpler test case than 'recursively grep /usr/share' since that's not necessarily reliable for any given state of the base system. It's kind of painful to trigger because it writes past the end of lnbuf on the heap for any terminating line length > LNBUFBUMP, but it has to write far enough off the end to anger the kernel. That last part is what I'm having trouble reasoning about -- what can I really consider sufficient to write into bad areas?

[emaste]

Ah, good point. It may just be that there's no reasonable / convenient way to test the segfaulting case.