[PATCH 1/3] fts: clarify ISSET

[PATCH 1/3] fts: clarify ISSET

[Paul Eggert]

* lib/fts.c (ISSET): Refactor to clarify boolean usage.
---
 ChangeLog | 5 +++++
 lib/fts.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 576db7231d..08436174d7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2022-12-06  Paul Eggert  <eggert@cs.ucla.edu>
+
+	fts: clarify ISSET
+	* lib/fts.c (ISSET): Refactor to clarify boolean usage.
+
 2022-12-05  Bruno Haible  <bruno@clisp.org>
 
 	argp: Correct documentation.
diff --git a/lib/fts.c b/lib/fts.c
index 5811f6ea20..8d2a5d2d90 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -235,7 +235,7 @@ static int      fts_safe_changedir (FTS *, FTSENT *, int, const char *)
 #define STREQ(a, b)     (strcmp (a, b) == 0)
 
 #define CLR(opt)        (sp->fts_options &= ~(opt))
-#define ISSET(opt)      (sp->fts_options & (opt))
+#define ISSET(opt)      ((sp->fts_options & (opt)) != 0)
 #define SET(opt)        (sp->fts_options |= (opt))
 
 /* FIXME: FTS_NOCHDIR is now misnamed.
-- 
2.38.1

[PATCH 2/3] fts: omit goto break_without_closedir

[Paul Eggert]

* lib/fts.c (fts_build): Refactor to omit goto.
---
 ChangeLog | 3 +++
 lib/fts.c | 8 ++------
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 08436174d7..54e3a23457 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2022-12-06  Paul Eggert  <eggert@cs.ucla.edu>
 
+	fts: omit goto break_without_closedir
+	* lib/fts.c (fts_build): Refactor to omit goto.
+
 	fts: clarify ISSET
 	* lib/fts.c (ISSET): Refactor to clarify boolean usage.
 
diff --git a/lib/fts.c b/lib/fts.c
index 8d2a5d2d90..27354d39c8 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1433,6 +1433,7 @@ fts_build (register FTS *sp, int type)
                                 cur->fts_info = (continue_readdir || nitems)
                                                 ? FTS_ERR : FTS_DNR;
                         }
+                        closedir_and_clear(cur->fts_dirp);
                         break;
                 }
                 if (!ISSET(FTS_SEEDOT) && ISDOT(dp->d_name))
@@ -1553,15 +1554,10 @@ mem1:                           saved_errno = errno;
                         /* When there are too many dir entries, leave
                            fts_dirp open, so that a subsequent fts_read
                            can take up where we leave off.  */
-                        goto break_without_closedir;
+                        break;
                 }
         }
 
-        if (cur->fts_dirp)
-                closedir_and_clear(cur->fts_dirp);
-
- break_without_closedir:
-
         /*
          * If realloc() changed the address of the file name, adjust the
          * addresses for the rest of the tree and the dir list.
-- 
2.38.1

[PATCH 3/3] fts: fix race + mishandling of fstatat failure

[Paul Eggert]

I hope this fixes a Luke Dashjr coreutils bug report about ext4
ramdisks; see “9.1: du Aborted (corrupt filesystem)”
<https://debbugs.gnu.org/59821>.
* lib/fts.c (fts_build): Fix two bugs.  First, fts_stat was being
called without checking its return value, causing a later abort.
Second, there was a race between opening a directory and statting
it, fixed by using fstat on the file descriptor rather than
fstatat on the directory name.
---
 ChangeLog | 10 ++++++++++
 lib/fts.c | 32 ++++++++++++++++++++++++--------
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 54e3a23457..feab2d6ca4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2022-12-06  Paul Eggert  <eggert@cs.ucla.edu>
 
+	fts: fix race + mishandling of fstatat failure
+	I hope this fixes a Luke Dashjr coreutils bug report about ext4
+	ramdisks; see “9.1: du Aborted (corrupt filesystem)”
+	<https://debbugs.gnu.org/59821>.
+	* lib/fts.c (fts_build): Fix two bugs.  First, fts_stat was being
+	called without checking its return value, causing a later abort.
+	Second, there was a race between opening a directory and statting
+	it, fixed by using fstat on the file descriptor rather than
+	fstatat on the directory name.
+
 	fts: omit goto break_without_closedir
 	* lib/fts.c (fts_build): Refactor to omit goto.
 
diff --git a/lib/fts.c b/lib/fts.c
index 27354d39c8..74a08f7ec8 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1316,19 +1316,35 @@ fts_build (register FTS *sp, int type)
             /* Rather than calling fts_stat for each and every entry encountered
                in the readdir loop (below), stat each directory only right after
                opening it.  */
-            if (cur->fts_info == FTS_NSOK)
-              cur->fts_info = fts_stat(sp, cur, false);
-            else if (sp->fts_options & FTS_TIGHT_CYCLE_CHECK)
-              {
-                /* Now read the stat info again after opening a directory to
+            bool stat_optimization = cur->fts_info == FTS_NSOK;
+
+            if (stat_optimization
+                /* Also read the stat info again after opening a directory to
                    reveal eventual changes caused by a submount triggered by
                    the traversal.  But do it only for utilities which use
                    FTS_TIGHT_CYCLE_CHECK.  Therefore, only find and du
                    benefit/suffer from this feature for now.  */
-                LEAVE_DIR (sp, cur, "4");
-                fts_stat (sp, cur, false);
-                if (! enter_dir (sp, cur))
+                || ISSET (FTS_TIGHT_CYCLE_CHECK))
+              {
+                if (!stat_optimization)
+                  LEAVE_DIR (sp, cur, "4");
+                if (fstat (dir_fd, cur->fts_statp) != 0)
+                  {
+                    int fstat_errno = errno;
+                    closedir_and_clear (cur->fts_dirp);
+                    if (type == BREAD)
+                      {
+                        cur->fts_errno = fstat_errno;
+                        cur->fts_info = FTS_NS;
+                      }
+                    __set_errno (fstat_errno);
+                    return NULL;
+                  }
+                if (stat_optimization)
+                  cur->fts_info = FTS_D;
+                else if (! enter_dir (sp, cur))
                   {
+                    closedir_and_clear (cur->fts_dirp);
                     __set_errno (ENOMEM);
                     return NULL;
                   }
-- 
2.38.1