Subject: bug#18406: O_NOATIME patch

From: Paul Eggert <[email-masked]>
Date: Mon, 31 Aug 2020 19:21:06 -0700

On 9/11/14 1:13 PM, Paul Eggert wrote:

> Thanks, but there's no need for that; just have 'grep' complain if the option is 
> used and O_NOATIME == 0.

On looking into this more today, O_NOATIME seems to be just a best-effort thing 
as some GNU/Linux filesystems ignore it, so grep should just join the throng and 
not worry whether O_NOATIME actually works.

Also, the O_NOATIME support was withdrawn from fts a couple of years ago, so 
'grep -r' can't easily avoid updating atime on directories.

A patch is attached. I'm still of two minds about this. The efficiency argument 
for the new option is not as strong as it used to be, now that relatime has 
taken over on ext4 style filesystems. So the main argument is "I want to search 
through this directory but don't want it to count as an access"; although that's 
indeed a use case I'm not quite sure it's worth modifying 'grep' over. It 
doesn't seem to be worth using up a scarce option letter over, anyway, so the 
attached patch uses just a long option.
>From f6506ab5ee58f37adc2c28c9be84e13a7467d935 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Mon, 31 Aug 2020 19:13:21 -0700
Subject: [PATCH] grep: new --atime-preserve option

Suggested by starlight.2014q3 (Bug#18406).
* NEWS, doc/grep.texi (Other Options): Mention it.
* src/grep.c (ATIME_PRESERVE_OPTION): New constant.
(binary): Remove, replacing with ...
(open_flags): ... this new variable.  All uses changed.
The new variable also supports the new option.
(long_options, grepfile, grep-command_line_arg, usage, main):
Add support for new option.
---
 NEWS          |  5 +++++
 doc/grep.texi |  7 +++++++
 src/grep.c    | 32 +++++++++++++++++++++-----------
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/NEWS b/NEWS
index 5f4c0b4..14d436d 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,11 @@ GNU grep NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** New features
+
+  The new --atime-preserve option causes grep to not update the last
+  data access time of input data files, when possible.
+
 ** Changes in behavior
 
   The --files-without-match (-L) option has reverted to its behavior
diff --git a/doc/grep.texi b/doc/grep.texi
index 02b1968..06452f3 100644
--- a/doc/grep.texi
+++ b/doc/grep.texi
@@ -774,6 +774,13 @@ and @file{file2}.
 Use line buffering on output.
 This can cause a performance penalty.
 
+@item --atime-preserve
+@opindex --atime-preserve
+@cindex last data access
+If possible, do not update the last data access time of input data
+files.  This option is silently ignored on platforms that always
+update the timestamp.
+
 @item -U
 @itemx --binary
 @opindex -U
diff --git a/src/grep.c b/src/grep.c
index 5764b2a..58af6ae 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -416,7 +416,8 @@ static char const short_options[] =
 /* Non-boolean long options that have no corresponding short equivalents.  */
 enum
 {
-  BINARY_FILES_OPTION = CHAR_MAX + 1,
+  ATIME_PRESERVE_OPTION = CHAR_MAX + 1,
+  BINARY_FILES_OPTION,
   COLOR_OPTION,
   EXCLUDE_DIRECTORY_OPTION,
   EXCLUDE_OPTION,
@@ -438,6 +439,7 @@ static struct option const long_options[] =
   {"perl-regexp",     no_argument, NULL, 'P'},
   {"after-context", required_argument, NULL, 'A'},
   {"before-context", required_argument, NULL, 'B'},
+  {"atime-preserve", no_argument, NULL, ATIME_PRESERVE_OPTION},
   {"binary-files", required_argument, NULL, BINARY_FILES_OPTION},
   {"byte-offset", no_argument, NULL, 'b'},
   {"context", required_argument, NULL, 'C'},
@@ -1033,7 +1035,7 @@ static intmax_t pending;	/* Pending lines of output.
 static bool done_on_match;	/* Stop scanning file on first match.  */
 static bool exit_on_match;	/* Exit on first match.  */
 static bool dev_null_output;	/* Stdout is known to be /dev/null.  */
-static bool binary;		/* Use binary rather than text I/O.  */
+static int open_flags = O_RDONLY | O_NOCTTY;  /* Flags to open input with.  */
 
 static void
 nlscan (char const *lim)
@@ -1687,9 +1689,7 @@ open_symlink_nofollow_error (int err)
 static bool
 grepfile (int dirdesc, char const *name, bool follow, bool command_line)
 {
-  int oflag = (O_RDONLY | O_NOCTTY
-               | (IGNORE_DUPLICATE_BRANCH_WARNING
-                  (binary ? O_BINARY : 0))
+  int oflag = (open_flags
                | (follow ? 0 : O_NOFOLLOW)
                | (skip_devices (command_line) ? O_NONBLOCK : 0));
   int desc = openat_safer (dirdesc, name, oflag);
@@ -1888,7 +1888,13 @@ grep_command_line_arg (char const *arg)
   if (STREQ (arg, "-"))
     {
       filename = label;
-      if (binary)
+      if (open_flags & O_NOATIME)
+        {
+          int flags = fcntl (STDIN_FILENO, F_GETFL);
+          if (0 <= flags)
+            fcntl (STDIN_FILENO, F_SETFL, flags | O_NOATIME);
+        }
+      if (open_flags & O_BINARY)
         xset_binary_mode (STDIN_FILENO, O_BINARY);
       return grepdesc (STDIN_FILENO, true);
     }
@@ -1936,6 +1942,7 @@ Pattern selection and interpretation:\n"), getprogname ());
       printf (_("\
 \n\
 Miscellaneous:\n\
+      --atime-preserve      preserve last-access times on input files\n\
   -s, --no-messages         suppress error messages\n\
   -v, --invert-match        select non-matching lines\n\
   -V, --version             display version information and exit\n\
@@ -2532,8 +2539,7 @@ main (int argc, char **argv)
         break;
 
       case 'U':
-        if (O_BINARY)
-          binary = true;
+        open_flags |= O_BINARY;
         break;
 
       case 'u':
@@ -2581,13 +2587,13 @@ main (int argc, char **argv)
       case 'f':
         if (STREQ (optarg, "-"))
           {
-            if (binary)
+            if (open_flags & O_BINARY)
               xset_binary_mode (STDIN_FILENO, O_BINARY);
             fp = stdin;
           }
         else
           {
-            fp = fopen (optarg, binary ? "rb" : "r");
+            fp = fopen (optarg, open_flags & O_BINARY ? "rb" : "r");
             if (!fp)
               die (EXIT_TROUBLE, errno, "%s", optarg);
           }
@@ -2692,6 +2698,10 @@ main (int argc, char **argv)
         eolbyte = '\0';
         break;
 
+      case ATIME_PRESERVE_OPTION:
+        open_flags |= O_NOATIME;
+        break;
+
       case BINARY_FILES_OPTION:
         if (STREQ (optarg, "binary"))
           binary_files = BINARY_BINARY_FILES;
@@ -2911,7 +2921,7 @@ main (int argc, char **argv)
               ? - (directories == RECURSE_DIRECTORIES)
               : 0 <= filename_option);
 
-  if (binary)
+  if (open_flags & O_BINARY)
     xset_binary_mode (STDOUT_FILENO, O_BINARY);
 
   /* Prefer sysconf for page size, as getpagesize typically returns int.  */
-- 
2.17.1