Page MenuHomeFreeBSD

bsdgrep trivial improvement: define REG_NOSPEC to be a no-op if not defined, use REG_NOSPEC instead of magic number
ClosedPublic

Authored by kevans on Apr 18 2017, 2:50 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Sep 11, 6:38 AM
Unknown Object (File)
Tue, Sep 10, 2:07 AM
Unknown Object (File)
Sat, Sep 7, 1:35 PM
Unknown Object (File)
Sat, Sep 7, 5:04 AM
Unknown Object (File)
Wed, Sep 4, 11:34 AM
Unknown Object (File)
Sat, Aug 31, 4:58 AM
Unknown Object (File)
Fri, Aug 23, 3:47 AM
Unknown Object (File)
Thu, Aug 22, 10:13 PM
Subscribers
None

Details

Summary

At the moment, glibc's cflags are not up to 0020 in value. Future proof this by using REG_NOSPEC directly and defining it to 0 for a no-op in other implementations where it's not defined.

Clearly we can define REG_NOSPEC differently if we pair bsdgrep with another regex(3) implementation that uses a different name, so this is the safe route.

Test Plan

Ensure no regressions with Kyua test suite

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 8777
Build 9133: arc lint + arc unit

Event Timeline

I agree this is better than 0020. But maybe we should have GREP_FIXED be disabled if neither REG_NOSPEC or REG_LITERAL are defined?

Do we have positive and negative test cases for grep / fgrep / egrep already?

I agree this is better than 0020. But maybe we should have GREP_FIXED be disabled if neiter REG_NOSPEC or REG_LITERAL are defined?

I considered this, but I've never actually seen an implementation that defined REG_LITERAL -- after reading the commment, I assumed it was a typo since I couldn't find any history of a REG_LITERAL in FreeBSDs regex.h and I hadn't seen an implementation that used it. A quick Google search revealed that I was definitively wrong. =)

Should I use this method of #define REG_LITERAL 0 if not defined and just set cflags |= REG_NOSPEC | REG_LITERAL, or would it be cleaner to have two #ifdef blocks inline in this switch? I personally prefer the former, but if there's a precedent it would be good to follow it.

Do we have positive and negative test cases for grep / fgrep / egrep already?

Heh, well, not really. We don't actually even have a single test that uses fgrep/grep -F. egrep has a primitive test for basic branching functionality/matching escaped characters, but the rest of its inclusion was incidental, having come from the one of the PRs for general matching broken-ness with the -o flag.

We could still use improvement in this area. =)

  • Error out at runtime on GREP_FIXED if literal expressions are not supported in regex(3)

Other cruft snuck in despite a recent merge with -HEAD...

Grrr, arc's behavior with Git is not friendly. Diffs against origin/master, not master

usr.bin/grep/grep.c
718

This won't work as DEFINED (upper-case) would be treated as a macro. We want #elif defined(), and I'll make the REG_NOSPEC case use defined() as well, for consistency.

usr.bin/grep/grep.c
721

also we want errx here - there is no errno to report.

This revision was automatically updated to reflect the committed changes.