[Toybox] [PATCH] grep: cleanup, add -Hs

Strake strake888 at gmail.com
Tue Jul 23 18:22:08 PDT 2013


On 20/07/2013, Isaac <idunham at lavabit.com> wrote:
>> @@ -60,10 +58,12 @@
>>
>>      while (regexec (&re, y, 1, &match, atBOL ? 0 : REG_NOTBOL) == 0) {
>>        if (atBOL) nMatch++;
>> -      c = 0; atBOL = 0;
>> +      toys.exitval = 0;
>> +      atBOL = 0;
>>        switch (TT.mode) {
>>        case 'q':
>> -        exit (0);
>> +        toys.exitval = 0;
>
> I don't think switch(TT.mode) is changing toys.exitval.

Ah, yes.

>> +        xexit ();
>>        case 'l':
>>          if (!(toys.optflags & FLAG_h)) printf ("%s\n", name);
>>          free (x);
>> @@ -121,7 +121,8 @@
>>      for (;;) {
>>        if (getline (&x, &l, f) < 0) {
>>          if (feof (f)) break;
>> -        err (2, "failed to read");
>> +        toys.exitval = 2;
>> +        perror_exit ("failed to read");
>
> I'd think this should be "warn and see if there are more REs"...
> But I could be wrong.

I'm not quite sure, but to carry on sans some given REs could give a
wrong answer, which I deemed worse than none at all.

> And this reminds me:
> Any particular reason that buildRE() needs to be a one-call monster that
> does all the regexes in a single pass?

No.

> I would have thought that the code used to parse TT.eArgu could be shared
> with parsing each line of TT.fArgu.

Done.

>> @@ -144,7 +148,8 @@
>>    if (regcomp (&re, re_xs,
>>                 (toys.optflags & (FLAG_E | FLAG_F) ? REG_EXTENDED : 0) |
>>                 (toys.optflags &  FLAG_i           ? REG_ICASE    : 0)) !=
>> 0) {
>
> You can remove the != 0.

So can the compiler.

# HG changeset patch
# User Strake
# Date 1374628771 18000
# Node ID 8ad85a95f7c3b75214fbd3a46fe834de45c39417
# Parent  019f54d50c8be39306675e0f182a124f6446307d
grep

diff -r 019f54d50c8b -r 8ad85a95f7c3 toys/pending/grep.c
--- a/toys/pending/grep.c	Mon Jul 22 01:54:28 2013 -0500
+++ b/toys/pending/grep.c	Tue Jul 23 20:19:31 2013 -0500
@@ -5,13 +5,13 @@
  * See http://pubs.opengroup.org/onlinepubs/9699919799/utilities/
  * See http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic/cmdbehav.html

-USE_GREP(NEWTOY(grep, "EFhinovclqe*f*m#", TOYFLAG_BIN))
+USE_GREP(NEWTOY(grep, "EFHahinosvclqe*f*m#", TOYFLAG_BIN))

 config GREP
   bool "grep"
   default n
   help
-    usage: grep [-clq] [-EFhinov] (-e RE | -f REfile | RE) [file...]
+    usage: grep [-clq] [-EFHhinosv] (-e RE | -f REfile | RE) [file...]

     modes:
       default: print lines from each file what match regular expression RE.
@@ -22,27 +22,25 @@
     flags:
       -E:   extended RE syntax
       -F:   fixed RE syntax, i.e. all characters literal
+      -H:   print file name
       -h:   not print file name
       -i:   case insensitive
       -n:   print line numbers
       -o:   print only matching part
+      -s:   keep silent on error
       -v:   invert match
 */

 #define FOR_grep
 #include "toys.h"
 #include <regex.h>
-#include <err.h>
-
-/* could be in GLOBALS but so need initialization code */
-static int c = 1;

 static regex_t re; /* fails in GLOBALS */

 GLOBALS(
   long mArgu;
   struct arg_list *fArgu, *eArgu;
-  char mode;
+  char mode, *re_xs;
 )

 static void do_grep (int fd, char *name) {
@@ -60,10 +58,11 @@

     while (regexec (&re, y, 1, &match, atBOL ? 0 : REG_NOTBOL) == 0) {
       if (atBOL) nMatch++;
-      c = 0; atBOL = 0;
+      toys.exitval = 0;
+      atBOL = 0;
       switch (TT.mode) {
       case 'q':
-        exit (0);
+        xexit ();
       case 'l':
         if (!(toys.optflags & FLAG_h)) printf ("%s\n", name);
         free (x);
@@ -102,15 +101,15 @@
   return re_ys;
 }

+void addRE (char *x) {
+  if (toys.optflags & FLAG_F) x = regfix (x);
+  if (TT.re_xs) TT.re_xs = astrcat (TT.re_xs, "|");
+  TT.re_xs = astrcat (TT.re_xs, x);
+  if (toys.optflags & FLAG_F) free (x);
+}
+
 void buildRE (void) {
-  char *re_xs;
-
-  re_xs = 0;
-  for (; TT.eArgu; TT.eArgu = TT.eArgu -> next) {
-    if (toys.optflags & FLAG_F) TT.eArgu -> arg = regfix (TT.eArgu -> arg);
-    if (re_xs) re_xs = xastrcat (re_xs, "|");
-    re_xs = xastrcat (re_xs, TT.eArgu -> arg);
-  }
+  for (; TT.eArgu; TT.eArgu = TT.eArgu -> next) addRE (TT.eArgu -> arg);
   for (; TT.fArgu; TT.fArgu = TT.fArgu -> next) {
     FILE *f;
     char *x, *y;
@@ -121,30 +120,32 @@
     for (;;) {
       if (getline (&x, &l, f) < 0) {
         if (feof (f)) break;
-        err (2, "failed to read");
+        toys.exitval = 2;
+        perror_exit ("failed to read");
       }
       y = x + strlen (x) - 1;
       if (y[0] == '\n') y[0] = 0;

-      y = toys.optflags & FLAG_F ? regfix (x) : x;
-      if (re_xs) re_xs = xastrcat (re_xs, "|");
-      re_xs = xastrcat (re_xs, y);
-      free (y);
+      addRE (x);
     }
     free (x);
     fclose (f);
   }

-  if (!re_xs) {
-    if (toys.optc < 1) errx (2, "no RE");
-    re_xs = toys.optflags & FLAG_F ? regfix (toys.optargs[0]) :
toys.optargs[0];
+  if (!TT.re_xs) {
+    if (toys.optc < 1) {
+      toys.exitval = 2;
+      error_exit ("no RE");
+    }
+    TT.re_xs = toys.optflags & FLAG_F ? regfix (toys.optargs[0]) :
toys.optargs[0];
     toys.optc--; toys.optargs++;
   }

-  if (regcomp (&re, re_xs,
+  if (regcomp (&re, TT.re_xs,
                (toys.optflags & (FLAG_E | FLAG_F) ? REG_EXTENDED : 0) |
                (toys.optflags &  FLAG_i           ? REG_ICASE    : 0)) != 0) {
-    errx (2, "bad RE");
+    toys.exitval = 2;
+    error_exit ("bad RE");
   }
 }

@@ -155,8 +156,9 @@
   if (toys.optflags & FLAG_l) TT.mode = 'l';
   if (toys.optflags & FLAG_q) TT.mode = 'q';

-  if (toys.optc > 0) loopfiles (toys.optargs, do_grep);
-  else do_grep (0, "-");
+  if (!(toys.optflags & FLAG_H) && (toys.optc < 2)) toys.optflags |= FLAG_h;

-  exit (c);
+  toys.exitval = 1;
+  loopfiles_rw (toys.optargs, O_RDONLY, 0, toys.optflags & FLAG_s, do_grep);
+  xexit ();
 }

 1374628928.0


More information about the Toybox mailing list