On Sat, Nov 04, 2017 at 01:55:51AM CDT, Paul Eggert wrote:
Zev Weiss wrote:
I'm wondering how robust (paranoid?) grep really needs to be in the
face of concurrent filesystem modifications.
Not that paranoid. Still, it's better for grep to avoid races when
it's easy, as is the case here.
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.
Zev
>From 0f350d2b5814f425f1c2a100fbec4d017ceb4d8b Mon Sep 17 00:00:00 2001
From: Zev Weiss <z...@bewilderbeest.net>
Date: Sat, 4 Nov 2017 15:32:23 -0500
Subject: [PATCH] grep: simplify out_file handling
* src/grep.c (print_filenames): New tristate enum (-H, -h, or
neither); supplants with_filenames and no_filenames.
(single_command_line_arg): New variable indicating if grep was run
with a single command-line argument.
(no_filenames): Remove variable.
(grepdirent): Don't twiddle out_file back and forth during recursion.
(grepdesc): Turn off out_file on 'grep -r foo nondirectory'.
(main): Replace with_filenames and no_filenames with print_filenames.
Enable out_file when both -r/-R and multiple arguments are given.
---
src/grep.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/src/grep.c b/src/grep.c
index 2277a60..3d31d8a 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -1001,16 +1001,25 @@ static enum
LISTFILES_NONMATCHING,
} list_files;
+/* What was requested on the command-line w.r.t. printing filenames. */
+static enum
+{
+ PRINTFILENAMES_DEFAULT, /* neither -h nor -H */
+ PRINTFILENAMES_ON, /* -H */
+ PRINTFILENAMES_OFF, /* -h */
+} print_filenames;
+
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 single_command_line_arg; /* True if we received exactly one
+ command-line argument */
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. */
@@ -1590,11 +1599,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,
@@ -1615,10 +1620,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;
@@ -1781,6 +1783,12 @@ grepdesc (int desc, bool command_line)
&& skipped_file (filename, true, S_ISDIR (st.st_mode) != 0))
goto closeout;
+ /* Don't print file names if invoked as 'grep -r foo <non-directory>'. */
+ if (command_line && single_command_line_arg
+ && print_filenames == PRINTFILENAMES_DEFAULT
+ && directories == RECURSE_DIRECTORIES && !S_ISDIR (st.st_mode))
+ out_file = 0;
+
if (desc != STDIN_FILENO
&& directories == RECURSE_DIRECTORIES && S_ISDIR (st.st_mode))
{
@@ -2416,7 +2424,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;
@@ -2506,8 +2513,7 @@ main (int argc, char **argv)
break;
case 'H':
- with_filenames = true;
- no_filenames = false;
+ print_filenames = PRINTFILENAMES_ON;
break;
case 'I':
@@ -2599,8 +2605,7 @@ main (int argc, char **argv)
break;
case 'h':
- with_filenames = false;
- no_filenames = true;
+ print_filenames = PRINTFILENAMES_OFF;
break;
case 'i':
@@ -2885,7 +2890,10 @@ main (int argc, char **argv)
&match_size, NULL) == 0)
== out_invert);
- if ((argc - optind > 1 && !no_filenames) || with_filenames)
+ single_command_line_arg = argc - optind == 1;
+ if (print_filenames == PRINTFILENAMES_ON
+ || (print_filenames == PRINTFILENAMES_DEFAULT
+ && (argc - optind > 1 || directories == RECURSE_DIRECTORIES)))
out_file = 1;
if (binary)
--
2.15.0