rmdir: fix --ignore-fail-on-non-empty with permissions errors

Since v6.10-21-ged5c4e7 `rmdir --ignore-fail-on-non-empty`
had reversed the failure status for directories that failed
to be removed for permissions reasons.  I.E. it would have
returned a failure status for such non empty dirs, and vice versa.

* src/rmdir.c (errno_may_be_non_empty): Rename from the
more confusing errno_may_be_empty(), and remove the EEXIST
case (specific to Solaris), which is moot here since
handled in errno_rmdir_non_empty().
(ignorable_failure): Fix the logic error so that
_non_ empty dirs are deemed to have ignorable failures.
(main): Fix clobbering of errno by is_empty_dir().
(remove_parents): Likewise.
* tests/rmdir/ignore.sh: Add a test case.
* THANKS.in: Add reporter who fixed the errno handling.
* NEWS: Mention the bug fix.
Fixes https://bugs.gnu.org/39364

diff --git a/NEWS b/NEWS
index 5231b84ac37e9f31e9100b01a8f5a3a9e84aa852..8a349634e7189087cffdf06aa34099c2ca152888 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   (like Solaris 10 and Solaris 11).
   [bug introduced in coreutils-8.31]
 
+  rmdir --ignore-fail-on-non-empty now works correctly for directories
+  that fail to be removed due to permission issues.  Previously the exit status
+  was reversed, failing for non empty and succeeding for empty directories.
+  [bug introduced in coreutils-6.11]
+
   'shuf -r -n 0 file' no longer mistakenly reads from standard input.
   [bug introduced with the --repeat feature in coreutils-8.22]
 

diff --git a/THANKS.in b/THANKS.in
index 23b089754ce5c72cc002c3353159de8883eea80a..48f6c04c72143032f44d0c7b61a821080bc770a3 100644 (file)
--- a/THANKS.in
+++ b/THANKS.in
@@ -421,6 +421,7 @@ Matthew Arnison                     maffew@cat.org.au
 Matthew Braun                       matthew@ans.net
 Matthew Clarke                      Matthew_Clarke@mindlink.bc.ca
 Matthew M. Boedicker                matthewm@boedicker.org
+Matthew Pfeiffer                    spferical@gmail.com
 Matthew S. Levine                   mslevine@theory.lcs.mit.edu
 Matthew Smith                       matts@bluesguitar.org
 Matthew Swift                       swift@alum.mit.edu

diff --git a/src/rmdir.c b/src/rmdir.c
index c9f417957e9acd83219eaa7520d0e6708eb7cf44..7301db5ee3c1a46bc7a0ecb043c36b5a45652503 100644 (file)
--- a/src/rmdir.c
+++ b/src/rmdir.c
@@ -77,16 +77,15 @@ errno_rmdir_non_empty (int error_number)
 }
 
 /* Return true if when rmdir fails with errno == ERROR_NUMBER
-   the directory may be empty.  */
+   the directory may be non empty.  */
 static bool
-errno_may_be_empty (int error_number)
+errno_may_be_non_empty (int error_number)
 {
   switch (error_number)
     {
     case EACCES:
     case EPERM:
     case EROFS:
-    case EEXIST:
     case EBUSY:
       return true;
     default:
@@ -101,8 +100,9 @@ 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))));
+              || (errno_may_be_non_empty (error_number)
+                  && ! is_empty_dir (AT_FDCWD, dir)
+                  && errno == 0 /* definitely non empty  */)));
 }
 
 /* Remove any empty parent directories of DIR.
@@ -133,18 +133,19 @@ remove_parents (char *dir)
         prog_fprintf (stdout, _("removing directory, %s"), quoteaf (dir));
 
       ok = (rmdir (dir) == 0);
+      int rmdir_errno = errno;
 
-      if (!ok)
+      if (! ok)
         {
           /* Stop quietly if --ignore-fail-on-non-empty. */
-          if (ignorable_failure (errno, dir))
+          if (ignorable_failure (rmdir_errno, dir))
             {
               ok = true;
             }
           else
             {
               /* Barring race conditions, DIR is expected to be a directory.  */
-              error (0, errno, _("failed to remove directory %s"),
+              error (0, rmdir_errno, _("failed to remove directory %s"),
                      quoteaf (dir));
             }
           break;
@@ -233,12 +234,13 @@ main (int argc, char **argv)
 
       if (rmdir (dir) != 0)
         {
-          if (ignorable_failure (errno, dir))
+          int rmdir_errno = errno;
+          if (ignorable_failure (rmdir_errno, dir))
             continue;
 
           /* Here, the diagnostic is less precise, since we have no idea
              whether DIR is a directory.  */
-          error (0, errno, _("failed to remove %s"), quoteaf (dir));
+          error (0, rmdir_errno, _("failed to remove %s"), quoteaf (dir));
           ok = false;
         }
       else if (remove_empty_parents)

diff --git a/src/system.h b/src/system.h
index 9d777680c259abf697b6c8946fe621ac7257182b..ebf68349ab17c77248f6107fd0deb27589b8c478 100644 (file)
--- a/src/system.h
+++ b/src/system.h
@@ -285,7 +285,9 @@ readdir_ignoring_dot_and_dotdot (DIR *dirp)
     }
 }
 
-/* Return true if DIR is determined to be an empty directory.  */
+/* Return true if DIR is determined to be an empty directory.
+   Return false with ERRNO==0 if DIR is a non empty directory.
+   Return false if not able to determine if directory empty.  */
 static inline bool
 is_empty_dir (int fd_cwd, char const *dir)
 {
@@ -310,6 +312,7 @@ is_empty_dir (int fd_cwd, char const *dir)
   dp = readdir_ignoring_dot_and_dotdot (dirp);
   saved_errno = errno;
   closedir (dirp);
+  errno = saved_errno;
   if (dp != NULL)
     return false;
   return saved_errno == 0 ? true : false;

diff --git a/tests/rmdir/ignore.sh b/tests/rmdir/ignore.sh
index d000c30cbbff883a2f3507f75863f5ec7b08f708..65e92d012c21d689d6babbc5f278812a43f2238b 100755 (executable)
--- a/tests/rmdir/ignore.sh
+++ b/tests/rmdir/ignore.sh
@@ -29,4 +29,21 @@ test -d "$cwd/a/x" || fail=1
 test -d "$cwd/a/b" && fail=1
 test -d "$cwd/a/b/c" && fail=1
 
+# Ensure that with --ignore-fail-on-non-empty, we still fail, e.g., for EPERM.
+# Between 6.11 and 8.31, the following rmdir would mistakenly succeed.
+mkdir -p x/y || framework_failure_
+chmod a-w x || framework_failure_
+returns_ 1 rmdir --ignore-fail-on-non-empty x/y || fail=1
+test -d x/y || fail=1
+# Between 6.11 and 8.31, the following rmdir would mistakenly fail,
+# and also give a non descript error
+touch x/y/z || framework_failure_
+rmdir --ignore-fail-on-non-empty x/y || fail=1
+test -d x/y || fail=1
+# assume empty dir if unreadable entries (so failure to remove diagnosed)
+rm x/y/z || framework_failure_
+chmod a-r x/y || framework_failure_
+returns_ 1 rmdir --ignore-fail-on-non-empty x/y || fail=1
+test -d x/y || fail=1
+
 Exit $fail