FYI: cycle-check bug: the next step

[Jim Meyering]

There was a weakness in the cycle-check module, described here:

  http://article.gmane.org/gmane.comp.gnu.core-utils.bugs/6600

I've just fixed fts.c (via the changes to fts-cycle.c, see below)
so that it too avoids the problem.

At the same time, I've factored the simple definition of SAME_INODE
into its own file.  It's just a 3-line definition, but those three
lines appeared in so many places that I felt I had to do it, even
if it meant modifying a handful of .m4 files each to `require' the
new .h file.

lib/ChangeLog
2006-03-12  Jim Meyering  <address@hidden>

        * fts-cycle.c (leave_dir): If cycle-check's saved dev-ino pair matches
        that of the current directory (which we're about to chdir ".." out of),
        then save the dev-ino of the parent, instead.

        * same-inode.h (SAME_INODE): New file/macro.
        * chdir-safer.c (SAME_INODE): Remove definition.
        Include "same-inode.h", instead.
        * same.c: Likewise.
        * cycle-check.h: Include "same-inode.h".
        (CYCLE_CHECK_REFLECT_CHDIR_UP): Define.
        * cycle-check.c (SAME_INODE): Remove definition.
        * root-dev-ino.h: Include "same-inode.h".

ChangeLog
2006-03-12  Jim Meyering  <address@hidden>

        * src/remove.c (AD_pop_and_chdir): Use new macro,
        CYCLE_CHECK_REFLECT_CHDIR_UP, rather than open-coding it.

        * src/system.h (SAME_INODE): Remove definition.
        Include "same-inode.h", instead.

m4/ChangeLog
2006-03-12  Jim Meyering  <address@hidden>

        * chdir-safer.m4 (gl_CHDIR_SAFER): Add same-inode.h to the list.
        * cycle-check.m4 (gl_CYCLE_CHECK): Likewise.
        * same.m4 (gl_SAME): Likewise.
        * root-dev-ino.m4 (gl_ROOT_DEV_INO): Likewise.

Here are selected diffs:

Index: lib/cycle-check.h
===================================================================
RCS file: /fetish/cu/lib/cycle-check.h,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -p -u -r1.6 -r1.7
--- lib/cycle-check.h   14 May 2005 07:58:06 -0000      1.6
+++ lib/cycle-check.h   12 Mar 2006 22:01:29 -0000      1.7
@@ -1,6 +1,6 @@
 /* help detect directory cycles efficiently
 
-   Copyright (C) 2003, 2004 Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2006 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -30,6 +30,7 @@
 # endif
 # include <stdbool.h>
 # include "dev-ino.h"
+# include "same-inode.h"
 
 struct cycle_check_state
 {
@@ -41,4 +42,15 @@ struct cycle_check_state
 void cycle_check_init (struct cycle_check_state *state);
 bool cycle_check (struct cycle_check_state *state, struct stat const *sb);
 
+# define CYCLE_CHECK_REFLECT_CHDIR_UP(State, SB_dir, SB_subdir)        \
+  do                                                           \
+    {                                                          \
+      if (SAME_INODE ((State)->dev_ino, SB_subdir))            \
+       {                                                       \
+         (State)->dev_ino.st_dev = (SB_dir).st_dev;            \
+         (State)->dev_ino.st_ino = (SB_dir).st_ino;            \
+       }                                                       \
+    }                                                          \
+  while (0)
+
 #endif
Index: lib/fts-cycle.c
===================================================================
RCS file: /fetish/cu/lib/fts-cycle.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -p -u -r1.4 -r1.5
--- lib/fts-cycle.c     27 Sep 2005 08:58:55 -0000      1.4
+++ lib/fts-cycle.c     12 Mar 2006 22:03:17 -0000      1.5
@@ -1,6 +1,6 @@
 /* Detect cycles in file tree walks.
 
-   Copyright (C) 2003, 2004, 2005 Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2005, 2006 Free Software Foundation, Inc.
 
    Written by Jim Meyering.
 
@@ -129,9 +129,9 @@ enter_dir (FTS *fts, FTSENT *ent)
 static void
 leave_dir (FTS *fts, FTSENT *ent)
 {
+  struct stat const *st = ent->fts_statp;
   if (fts->fts_options & (FTS_TIGHT_CYCLE_CHECK | FTS_LOGICAL))
     {
-      struct stat const *st = ent->fts_statp;
       struct Active_dir obj;
       void *found;
       obj.dev = st->st_dev;
@@ -141,6 +141,13 @@ leave_dir (FTS *fts, FTSENT *ent)
        abort ();
       free (found);
     }
+  else
+    {
+      FTSENT *parent = ent->fts_parent;
+      if (parent != NULL)
+       CYCLE_CHECK_REFLECT_CHDIR_UP (fts->fts_cycle.state,
+                                     *(parent->fts_statp), *st);
+    }
 }
 
Index: src/remove.c
===================================================================
RCS file: /fetish/cu/src/remove.c,v
retrieving revision 1.147
retrieving revision 1.148
diff -u -p -u -r1.147 -r1.148
--- src/remove.c        10 Mar 2006 21:47:34 -0000      1.147
+++ src/remove.c        12 Mar 2006 22:21:05 -0000      1.148
@@ -411,8 +411,8 @@ AD_pop_and_chdir (DIR **dirp, Dirstack_s
      could be reused in the creation (by some other process)
      of a directory that this rm process would encounter,
      which would result in a false-positive cycle indication.  */
-  if (SAME_INODE (ds->cycle_check_state.dev_ino, leaf_dev_ino))
-    ds->cycle_check_state.dev_ino = top->dev_ino;
+  CYCLE_CHECK_REFLECT_CHDIR_UP (&ds->cycle_check_state,
+                               top->dev_ino, leaf_dev_ino);
 
   /* Propagate any failure to parent.  */
   UPDATE_STATUS (top->status, old_status);