4701 would like grep context options (-A, -B, -C)

Review Request #550 — Created June 1, 2017 and submitted

mbarden
illumos-gate
4701
general

4701 would like grep context options (-A, -B, -C)

See tests in $SRC/test/util_tests/tests/grep_xpg4

  • 0
  • 0
  • 3
  • 0
  • 3
Description From Last Updated
rm
  1. 
      
  2. usr/src/cmd/grep_xpg4/grep.c (Diff revision 1)
     
     
    While this was mentioned on the thread, I think we should really have a comment here that better explains why we're doing this assignment in this style to a pointer. Maybe it may make sense to actually be a pointer to a static PATTERN structure that's initialized globally and basically at least a valid thing to dereference?
  3. usr/src/cmd/grep_xpg4/grep.c (Diff revision 1)
     
     
    Sorry, I didn't notice this earlier, but this synopsis isn't correct. It's not an or of any of these options, each can be specified separately. So I think it needs to really be [-A num] [-B num] [-C num], etc. I also think using num like we do in the manual page will be clearer than the '#' character.
  4. usr/src/man/man1/grep.1 (Diff revision 1)
     
     
    The way the synopsis is done here feels a little confusing. It makes it seem like it's either -C or -# followed by a number which will trigger the behavior. I'm not sure I have a better suggestion as to what put here at the moment yet though. Maybe a separate [-number] would work?
  5. 
      
mbarden
rm
  1. Thanks for the follow ups, Matt. I apprecaite it. Let's go ahead and get this RTI'd.

  2. 
      
jgmills
  1. In general, I like the test idea.  I assume that this version of grep passes all the tests.
    
    I found a few peculiarities in usr/src/cmd/grep_xpg4/grep.c, none of which should affect the test results:
    
    At line 188, the comments seems misplaced.  I would expect it just above the function call that it references.
    
    At lines 322 and 338, I see an explicit number 10.  Perhaps you could replace that with a more meaningful symbol.
    
    At line 910, it might be better to specify: conmatches != 0 .  To make it even clearer, you could define nearmatch as a boolian type.  Then you could initialize it with: nearmatch = (conmatches != 0) .
    
    At lines 958 and 993, the variable `eof' could be a boolian, making the code simpler.
    
    Other than these minor things, please continue with the RTI process.
  2. 
      
gwr
  1. Ship It!
  2. 
      
mbarden
mbarden
Review request changed

Status: Closed (submitted)

Loading...