When moving "up" the hierarchy, be careful to remove a just-emptied

directory before opening ".", to avoid trouble with file system
implementations that cache readdir results at opendir-time.
* src/remove.c (AD_pop_and_chdir): Add a file descriptor parameter.
Don't update **DIRP.  Don't call fdopendir here.
(remove_dir): Call fdopendir here instead.
Report and patch from Mikulas Patocka:
<http://lists.gnu.org/archive/html/bug-coreutils/2006-12/msg00170.html>

diff --git a/ChangeLog b/ChangeLog
index 9e24a1c..658076e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2006-12-28  Jim Meyering  <jim@meyering.net>
+
+       When moving "up" the hierarchy, be careful to remove a just-emptied
+       directory before opening ".", to avoid trouble with file system
+       implementations that cache readdir results at opendir-time.
+       * src/remove.c (AD_pop_and_chdir): Add a file descriptor parameter.
+       Don't update **DIRP.  Don't call fdopendir here.
+       (remove_dir): Call fdopendir here instead.
+       Report and patch from Mikulas Patocka:
+       <http://lists.gnu.org/archive/html/bug-coreutils/2006-12/msg00170.html>
+
 2006-12-27  Jim Meyering  <jim@meyering.net>
 
        * src/tail.c (usage): Mention +N for --bytes and --lines.

diff --git a/src/remove.c b/src/remove.c
index f6d3f0c..68cb2d1 100644 (file)
--- a/src/remove.c
+++ b/src/remove.c
@@ -432,7 +432,7 @@ ds_free (Dirstack_state *ds)
    directory (usually now empty) from which we're coming, and which
    corresponds to the input value of *DIRP.  */
 static char *
-AD_pop_and_chdir (DIR **dirp, Dirstack_state *ds)
+AD_pop_and_chdir (DIR **dirp, int *fdp, Dirstack_state *ds)
 {
   struct AD_ent *leaf_dir_ent = AD_stack_top(ds);
   struct dev_ino leaf_dev_ino = leaf_dir_ent->dev_ino;
@@ -499,15 +499,6 @@ AD_pop_and_chdir (DIR **dirp, Dirstack_state *ds)
        {
          error (0, 0, _("FATAL: directory %s changed dev/ino"),
                 quote (full_filename (".")));
-         goto close_and_next;
-       }
-
-      *dirp = fdopendir (fd);
-      if (*dirp == NULL)
-       {
-         error (0, errno, _("FATAL: cannot return to .. from %s"),
-                quote (full_filename (".")));
-
        close_and_next:;
          close (fd);
 
@@ -515,6 +506,7 @@ AD_pop_and_chdir (DIR **dirp, Dirstack_state *ds)
          free (prev_dir);
          longjmp (ds->current_arg_jumpbuf, 1);
        }
+      *fdp = fd;
     }
   else
     {
@@ -524,9 +516,10 @@ AD_pop_and_chdir (DIR **dirp, Dirstack_state *ds)
                 quote (full_filename (prev_dir)));
          goto next_cmdline_arg;
        }
-      *dirp = NULL;
+      *fdp = AT_FDCWD;
     }
 
+  *dirp = NULL;
   return prev_dir;
 }
 
@@ -1384,9 +1377,9 @@ remove_dir (int fd_cwd, Dirstack_state *ds, char const *dir,
       {
        /* The name of the directory that we have just processed,
           nominally removing all of its contents.  */
-       char *empty_dir = AD_pop_and_chdir (&dirp, ds);
-       int fd = (dirp != NULL ? dirfd (dirp) : AT_FDCWD);
-       assert (dirp != NULL || AD_stack_height (ds) == 1);
+       int fd;
+       char *empty_dir = AD_pop_and_chdir (&dirp, &fd, ds);
+       assert (fd != AT_FDCWD || AD_stack_height (ds) == 1);
 
        /* Try to remove EMPTY_DIR only if remove_cwd_entries succeeded.  */
        if (tmp_status == RM_OK)
@@ -1405,6 +1398,8 @@ remove_dir (int fd_cwd, Dirstack_state *ds, char const *dir,
              {
                free (empty_dir);
                status = s;
+               if (fd != AT_FDCWD)
+                 close (fd);
                goto closedir_and_return;
              }
 
@@ -1426,7 +1421,18 @@ remove_dir (int fd_cwd, Dirstack_state *ds, char const *dir,
 
        free (empty_dir);
 
-       if (AD_stack_height (ds) == 1)
+       if (fd != AT_FDCWD)
+         {
+           dirp = fdopendir (fd);
+           if (dirp == NULL)
+             {
+               error (0, errno, _("FATAL: cannot return to .. from %s"),
+                 quote (full_filename (".")));
+               close (fd);
+               longjmp (ds->current_arg_jumpbuf, 1);
+             }
+         }
+       else
          break;
       }
     }