Pádraig Brady wrote:

I suppose we could stat() and if that succeeds && !fifo, only then call open() ?
Patch to do that is attached.

Better is to use open with O_NONBLOCK, as this avoids interpreting the file name twice in the usual case. Better yet is to use O_PATH if available, as this avoids interpreting the file name twice even when the file is unreadable or is a FIFO that would block. Also, the comment just before the code should be changed to match the altered code. Proposed followup patch attached.

It is puzzling that df calls fstat or stat, when it should just be calling fstatfs or statfs. But that is a different matter.
From c02f0e6009d7d77ee7a8e1299e03ca10a477c9e4 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sun, 29 Oct 2017 23:09:49 -0700
Subject: [PATCH] df: improve fix for FIFOs

* src/df.c (main): Use O_PATH if available.  Otherwise,
use O_NONBLOCK.  Either way, this avoids the need to
interpret the file name twice.
---
 src/df.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/df.c b/src/df.c
index ee04d51..cab1958 100644
--- a/src/df.c
+++ b/src/df.c
@@ -1708,21 +1708,25 @@ main (int argc, char **argv)
       stats = xnmalloc (argc - optind, sizeof *stats);
       for (int i = optind; i < argc; ++i)
         {
-          /* Prefer to open with O_NOCTTY and use fstat, but fall back
-             on using "stat", in case the file is unreadable.  */
-          if (stat (argv[i], &stats[i - optind]))
+#if O_PATH
+          int oflag = O_PATH;
+          bool skip_stat = true;
+#else
+          /* Fall back on stat in case the file is unreadable or is a
+             FIFO that would block.  */
+          int oflag = O_RDONLY | O_NOCTTY | O_NONBLOCK;
+          bool skip_stat = false;
+#endif
+          int fd = open (argv[i], oflag);
+          if ((fd < 0 || fstat (fd, &stats[i - optind]) != 0)
+              && (skip_stat || stat (argv[i], &stats[i - optind]) != 0))
             {
               error (0, errno, "%s", quotef (argv[i]));
               exit_status = EXIT_FAILURE;
               argv[i] = NULL;
             }
-          else if (! S_ISFIFO (stats[i - optind].st_mode))
-            {
-              /* open() is needed to automount in some cases.  */
-              int fd = open (argv[i], O_RDONLY | O_NOCTTY);
-              if (0 <= fd)
-                close (fd);
-            }
+          if (0 <= fd)
+            close (fd);
         }
     }
 
-- 
2.7.4

Reply via email to