minor bug in rm: couldn't remove empty, inaccessible directory

[Jim Meyering]

I noticed a minor bug in rm (on both the trunk and the b5 branch).
Demonstrate with this:

  mkdir -m0 inacc && rm -rf inacc

Before, it would fail with permission denied.
Now, it removes it, just as well as `rmdir inacc' does.

Here's what I've done on the trunk:
I haven't fixed it on the branch, and don't plan to.

2006-02-11  Jim Meyering  <address@hidden>

        rm -r must remove an empty directory, even if it is inaccessible.
        * src/remove.c (close_preserve_errno): New function.
        (fd_to_subdirp): Don't print a diagnostic in this function.
        Do it from the callers instead, unless rmdir succeeds.
        (remove_cwd_entries, remove_dir): Adjust callers.
        * tests/rm/empty-inacc: New test for the above.
        * tests/rm/Makefile.am (TESTS): Add empty-inacc.
        * NEWS: Mention this bug fix.
        * tests/rm/rm2: Adjust two expected diagnostics, now that they're
        a tiny bit less precise: cannot remove `a/1': ... instead of
        cannot open directory `a/1': ...

Index: src/remove.c
===================================================================
RCS file: /fetish/cu/src/remove.c,v
retrieving revision 1.144
retrieving revision 1.145
diff -u -p -u -r1.144 -r1.145
--- src/remove.c        6 Jan 2006 10:14:19 -0000       1.144
+++ src/remove.c        11 Feb 2006 19:25:02 -0000      1.145
@@ -142,6 +142,16 @@ struct dirstack_state
 };
 typedef struct dirstack_state Dirstack_state;
 
+/* Just like close(fd), but don't modify errno. */
+static inline int
+close_preserve_errno (int fd)
+{
+  int saved_errno = errno;
+  int result = close (fd);
+  errno = saved_errno;
+  return result;
+}
+
 static void
 hash_freer (void *x)
 {
@@ -997,29 +1007,22 @@ fd_to_subdirp (int fd_cwd, char const *f
      to detect directory cycles.  */
   if (fd_sub < 0 || fstat (fd_sub, subdir_sb) != 0)
     {
-      if (errno != ENOENT || !x->ignore_missing_files)
-       error (0, errno,
-              _("cannot remove %s"), quote (full_filename (f)));
       if (0 <= fd_sub)
-       close (fd_sub);
+       close_preserve_errno (fd_sub);
       return NULL;
     }
 
   if (! S_ISDIR (subdir_sb->st_mode))
     {
-      error (0, prev_errno ? prev_errno : ENOTDIR, _("cannot remove %s"),
-            quote (full_filename (f)));
-      close (fd_sub);
+      errno = prev_errno ? prev_errno : ENOTDIR;
+      close_preserve_errno (fd_sub);
       return NULL;
     }
 
   DIR *subdir_dirp = fdopendir (fd_sub);
   if (subdir_dirp == NULL)
     {
-      if (errno != ENOENT || !x->ignore_missing_files)
-       error (0, errno, _("cannot open directory %s"),
-              quote (full_filename (f)));
-      close (fd_sub);
+      close_preserve_errno (fd_sub);
       return NULL;
     }
 
@@ -1110,6 +1113,11 @@ remove_cwd_entries (DIR **dirp,
                                              x, errno, subdir_sb, ds, NULL);
            if (subdir_dirp == NULL)
              {
+               /* CAUTION: this test and diagnostic are identical those
+                  following the other use of fd_to_subdirp.  */
+               if (errno != ENOENT || !x->ignore_missing_files)
+                 error (0, errno,
+                        _("cannot remove %s"), quote (full_filename (f)));
                AD_mark_as_unremovable (ds, f);
                status = RM_ERROR;
                break;
@@ -1188,7 +1196,26 @@ remove_dir (int fd_cwd, Dirstack_state *
   DIR *dirp = fd_to_subdirp (fd_cwd, dir, x, 0, &dir_sb, ds, cwd_errno);
 
   if (dirp == NULL)
-    return RM_ERROR;
+    {
+      int saved_errno = errno;
+      if (errno == EACCES)
+       {
+         /* If fd_to_subdirp fails due to permissions, then try to
+            remove DIR via rmdir, in case it's just an empty directory.  */
+         if (rmdir (dir) == 0)
+           return RM_OK;
+
+         errno = saved_errno;
+       }
+
+      /* CAUTION: this test and diagnostic are identical those
+        following the other use of fd_to_subdirp.  */
+      if (errno != ENOENT || !x->ignore_missing_files)
+       error (0, errno,
+              _("cannot remove %s"), quote (full_filename (dir)));
+
+      return RM_ERROR;
+    }
 
   if (ROOT_DEV_INO_CHECK (x->root_dev_ino, &dir_sb))
     {