On 11/4/17 2:00 PM, Zev Weiss wrote:

OK, how about the attached patch then -- it both avoids updating out_file during directory recursion (simpler, and perhaps a tiny performance win) and doesn't add any syscalls or races.

Thanks, I (finally) installed that cleanup patch, and followed it up with two more simplifying patches. The combined change looks like the attached.
diff --git a/src/grep.c b/src/grep.c
index 6f35f26..7f3ada1 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -1000,16 +1000,19 @@ static enum
   LISTFILES_NONMATCHING,
 } list_files;
 
+/* Whether to output filenames.  1 means yes, 0 means no, and -1 means
+   'grep -r PATTERN FILE' was used and it is not known yet whether
+   FILE is a directory (which means yes) or not (which means no).  */
+static int out_file;
+
 static int filename_mask;	/* If zero, output nulls after filenames.  */
 static bool out_quiet;		/* Suppress all normal output. */
 static bool out_invert;		/* Print nonmatching stuff. */
-static int out_file;		/* Print filenames. */
 static bool out_line;		/* Print line numbers. */
 static bool out_byte;		/* Print byte offsets. */
 static intmax_t out_before;	/* Lines of leading context. */
 static intmax_t out_after;	/* Lines of trailing context. */
 static bool count_matches;	/* Count matching lines.  */
-static bool no_filenames;	/* Suppress file names.  */
 static intmax_t max_count;	/* Max number of selected
                                    lines from an input file.  */
 static bool line_buffered;	/* Use line buffering.  */
@@ -1589,11 +1592,7 @@ grepdirent (FTS *fts, FTSENT *ent, bool command_line)
   command_line &= ent->fts_level == FTS_ROOTLEVEL;
 
   if (ent->fts_info == FTS_DP)
-    {
-      if (directories == RECURSE_DIRECTORIES && command_line)
-        out_file &= ~ (2 * !no_filenames);
-      return true;
-    }
+    return true;
 
   if (!command_line
       && skipped_file (ent->fts_name, false,
@@ -1614,10 +1613,7 @@ grepdirent (FTS *fts, FTSENT *ent, bool command_line)
     {
     case FTS_D:
       if (directories == RECURSE_DIRECTORIES)
-        {
-          out_file |= 2 * !no_filenames;
-          return true;
-        }
+        return true;
       fts_set (fts, ent, FTS_SKIP);
       break;
 
@@ -1780,6 +1776,10 @@ grepdesc (int desc, bool command_line)
       && skipped_file (filename, true, S_ISDIR (st.st_mode) != 0))
     goto closeout;
 
+  /* Don't output file names if invoked as 'grep -r PATTERN NONDIRECTORY'.  */
+  if (out_file < 0)
+    out_file = !!S_ISDIR (st.st_mode);
+
   if (desc != STDIN_FILENO
       && directories == RECURSE_DIRECTORIES && S_ISDIR (st.st_mode))
     {
@@ -2423,7 +2423,6 @@ main (int argc, char **argv)
   char *keys = NULL;
   size_t keycc = 0, oldcc, keyalloc = 0;
   int matcher = -1;
-  bool with_filenames = false;
   size_t cc;
   int opt, prepended;
   int prev_optind, last_recursive;
@@ -2433,6 +2432,10 @@ main (int argc, char **argv)
   exit_failure = EXIT_TROUBLE;
   initialize_main (&argc, &argv);
 
+  /* Which command-line options have been specified for filename output.
+     -1 for -h, 1 for -H, 0 for neither.  */
+  int filename_option = 0;
+
   eolbyte = '\n';
   filename_mask = ~0;
 
@@ -2514,8 +2517,7 @@ main (int argc, char **argv)
         break;
 
       case 'H':
-        with_filenames = true;
-        no_filenames = false;
+        filename_option = 1;
         break;
 
       case 'I':
@@ -2607,8 +2609,7 @@ main (int argc, char **argv)
         break;
 
       case 'h':
-        with_filenames = false;
-        no_filenames = true;
+        filename_option = -1;
         break;
 
       case 'i':
@@ -2898,8 +2899,10 @@ main (int argc, char **argv)
                                 &match_size, NULL) == 0)
                       == out_invert);
 
-  if ((argc - optind > 1 && !no_filenames) || with_filenames)
-    out_file = 1;
+  int num_operands = argc - optind;
+  out_file = (filename_option == 0 && num_operands <= 1
+              ? - (directories == RECURSE_DIRECTORIES)
+              : 0 <= filename_option);
 
   if (binary)
     xset_binary_mode (STDOUT_FILENO, O_BINARY);
@@ -2920,7 +2923,7 @@ main (int argc, char **argv)
     devices = READ_DEVICES;
 
   char *const *files;
-  if (optind < argc)
+  if (0 < num_operands)
     {
       files = argv + optind;
     }

Reply via email to