Re: rmdir --ignore-fail-on-non-empty fails with permission denied

[Jim Meyering]

address@hidden (Bob Proulx) wrote:
> Please maintain the CC to both the bug number and the mailing list
> when responding.  Thanks.
>
> Reported to the Debian BTS:
>
>   http://bugs.debian.org/350541
>
> 'rmdir --ignore-fail-on-non-empty' will ignore non-empty directories
> unless it has insufficient permission to remove them, in which case it
> fails.  Can rmdir avoid failing in this case?
>
> Here is a way to reproduce the issue.  Root access is required in
> order to have permission to set the immutable attribute.  The
> filesystem needs to be ext2-like in order to support it.
>
>   # mkdir testdir
>   # mkdir testdir/foo
>   # mkdir testdir/foo/bar
>   # mkdir testdir/foo/boo
>   # chattr +i testdir
>   # rmdir -p --ignore-fail-on-non-empty testdir/foo/bar ; echo $?
>   rmdir: testdir/foo: Permission denied
>   1

I've fixed it like this:
(but no test case yet -- volunteers welcome!)

        Improve "rmdir --ignore-fail-on-non-empty"
        * src/rmdir.c (remove_parents, main): With --ignore-fail-on-non-empty,
        suppress a diagnostic also for e.g., EACCES, which can happen
        with read-only media or when the parent directory has the immutable
        attribute (set via chattr +i).
        (errno_may_be_empty, ignorable_failure): New functions.
        * src/remove.c (is_empty_dir): Move function to ...
        * src/system.h (is_empty_dir): ...here, and make it inline.
        Suggested by Josselin Mouette in <http://bugs.debian.org/363011>
        via Bob Proulx.
        * NEWS: Mention the improvement.

Signed-off-by: Jim Meyering <address@hidden>
---
 ChangeLog    |   14 ++++++++++++++
 NEWS         |   10 +++++++++-
 THANKS       |    1 +
 src/remove.c |   32 +-------------------------------
 src/rmdir.c  |   36 +++++++++++++++++++++++++++++++-----
 src/system.h |   30 ++++++++++++++++++++++++++++++
 6 files changed, 86 insertions(+), 37 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5e15325..017c307 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2008-01-30  Jim Meyering  <address@hidden>
+
+       Improve "rmdir --ignore-fail-on-non-empty"
+       * src/rmdir.c (remove_parents, main): With --ignore-fail-on-non-empty,
+       suppress a diagnostic also for e.g., EACCES, which can happen
+       with read-only media or when the parent directory has the immutable
+       attribute (set via chattr +i).
+       (errno_may_be_empty, ignorable_failure): New functions.
+       * src/remove.c (is_empty_dir): Move function to ...
+       * src/system.h (is_empty_dir): ...here, and make it inline.
+       Suggested by Josselin Mouette in <http://bugs.debian.org/363011>
+       via Bob Proulx.
+       * NEWS: Mention the improvement.
+
 2008-01-29  Paul Eggert  <address@hidden>

        Don't modify argv in dd.
diff --git a/NEWS b/NEWS
index f474141..0d2d97d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,12 +1,20 @@
 GNU coreutils NEWS                                    -*- outline -*-

-* Noteworthy changes in release 6.10 (2008-01-22) [stable]
+* Noteworthy changes in release 6.?? (2008-??-??) [stable]

 ** Bug fixes

   ls no longer segfaults on files in /proc when linked with an older version
   of libselinux.  E.g., ls -l /proc/sys would dereference a NULL pointer.

+  "rmdir --ignore-fail-on-non-empty" detects and ignores the failure
+  in more cases when a directory is empty.
+
+
+* Noteworthy changes in release 6.10 (2008-01-22) [stable]
+
+** Bug fixes
+
   Fix a non-portable use of sed in configure.ac.
   [bug introduced in coreutils-6.9.92]

diff --git a/THANKS b/THANKS
index 1e04f9b..bb536b3 100644
--- a/THANKS
+++ b/THANKS
@@ -264,6 +264,7 @@ Joost van Baal                      address@hidden
 Jorge Stolfi                        address@hidden
 Joseph S. Myers                     address@hidden
 Joshua Hudson                       address@hidden
+Josselin Mouette                    address@hidden
 Juan F. Codagnone                   address@hidden
 Juan M. Guerrero                    address@hidden
 Jungshik Shin                       address@hidden
diff --git a/src/remove.c b/src/remove.c
index de8f5ff..fe603bb 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -1,5 +1,5 @@
 /* remove.c -- core functions for removing files and directories
-   Copyright (C) 88, 90, 91, 1994-2007 Free Software Foundation, Inc.
+   Copyright (C) 88, 90, 91, 1994-2008 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
@@ -726,36 +726,6 @@ AD_is_removable (Dirstack_state const *ds, char const 
*file)
   return ! (top->unremovable && hash_lookup (top->unremovable, file));
 }

-/* Return true if DIR is determined to be an empty directory.  */
-static bool
-is_empty_dir (int fd_cwd, char const *dir)
-{
-  DIR *dirp;
-  struct dirent const *dp;
-  int saved_errno;
-  int fd = openat (fd_cwd, dir,
-                  (O_RDONLY | O_DIRECTORY
-                   | O_NOCTTY | O_NOFOLLOW | O_NONBLOCK));
-
-  if (fd < 0)
-    return false;
-
-  dirp = fdopendir (fd);
-  if (dirp == NULL)
-    {
-      close (fd);
-      return false;
-    }
-
-  errno = 0;
-  dp = readdir_ignoring_dot_and_dotdot (dirp);
-  saved_errno = errno;
-  closedir (dirp);
-  if (dp != NULL)
-    return false;
-  return saved_errno == 0 ? true : false;
-}
-
 /* Return -1 if FILE is an unwritable non-symlink,
    0 if it is writable or some other type of file,
    a positive error number if there is some problem in determining the answer.
diff --git a/src/rmdir.c b/src/rmdir.c
index 96aa9af..bb1a0c8 100644
--- a/src/rmdir.c
+++ b/src/rmdir.c
@@ -74,13 +74,41 @@ static struct option const longopts[] =

 /* Return true if ERROR_NUMBER is one of the values associated
    with a failed rmdir due to non-empty target directory.  */
-
 static bool
 errno_rmdir_non_empty (int error_number)
 {
   return (error_number == RMDIR_ERRNO_NOT_EMPTY);
 }

+/* Return true if when rmdir fails with errno == ERROR_NUMBER
+   the directory may be empty.  */
+static bool
+errno_may_be_empty (int error_number)
+{
+  switch (error_number)
+    {
+    case EACCES:
+    case EPERM:
+    case EROFS:
+    case EEXIST:
+    case EBUSY:
+      return true;
+    default:
+      return false;
+    }
+}
+
+/* Return true if an rmdir failure with errno == error_number
+   for DIR is ignorable.  */
+static bool
+ignorable_failure (int error_number, char const *dir)
+{
+  return (ignore_fail_on_non_empty
+         && (errno_rmdir_non_empty (error_number)
+             || (errno_may_be_empty (error_number)
+                 && is_empty_dir (AT_FDCWD, dir))));
+}
+
 /* Remove any empty parent directories of DIR.
    If DIR contains slash characters, at least one of them
    (beginning with the rightmost) is replaced with a NUL byte.
@@ -113,8 +141,7 @@ remove_parents (char *dir)
       if (!ok)
        {
          /* Stop quietly if --ignore-fail-on-non-empty. */
-         if (ignore_fail_on_non_empty
-             && errno_rmdir_non_empty (errno))
+         if (ignorable_failure (errno, dir))
            {
              ok = true;
            }
@@ -210,8 +237,7 @@ main (int argc, char **argv)

       if (rmdir (dir) != 0)
        {
-         if (ignore_fail_on_non_empty
-             && errno_rmdir_non_empty (errno))
+         if (ignorable_failure (errno, dir))
            continue;

          /* Here, the diagnostic is less precise, since we have no idea
diff --git a/src/system.h b/src/system.h
index 54c8a8b..b0b9545 100644
--- a/src/system.h
+++ b/src/system.h
@@ -384,6 +384,36 @@ readdir_ignoring_dot_and_dotdot (DIR *dirp)
     }
 }

+/* Return true if DIR is determined to be an empty directory.  */
+static inline bool
+is_empty_dir (int fd_cwd, char const *dir)
+{
+  DIR *dirp;
+  struct dirent const *dp;
+  int saved_errno;
+  int fd = openat (fd_cwd, dir,
+                  (O_RDONLY | O_DIRECTORY
+                   | O_NOCTTY | O_NOFOLLOW | O_NONBLOCK));
+
+  if (fd < 0)
+    return false;
+
+  dirp = fdopendir (fd);
+  if (dirp == NULL)
+    {
+      close (fd);
+      return false;
+    }
+
+  errno = 0;
+  dp = readdir_ignoring_dot_and_dotdot (dirp);
+  saved_errno = errno;
+  closedir (dirp);
+  if (dp != NULL)
+    return false;
+  return saved_errno == 0 ? true : false;
+}
+
 /* Factor out some of the common --help and --version processing code.  */

 /* These enum values cannot possibly conflict with the option values
--
1.5.4.rc5.1.ge6bfe