>From 3b379d77ec342b66a36d4f39435dccc121ca86eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Thu, 30 Jan 2020 20:46:40 -0500
Subject: [PATCH] rmdir: fix --ignore-fail-on-non-empty with permissions errors

Since 6.11 `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
---
 NEWS                  |  5 +++++
 THANKS.in             |  1 +
 src/rmdir.c           | 21 +++++++++++----------
 tests/rmdir/ignore.sh | 12 ++++++++++++
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 5231b84ac..8a349634e 100644
--- 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 23b089754..a667197ed 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -420,6 +420,7 @@ Matt Swift                          address@hidden
 Matthew Arnison                     address@hidden
 Matthew Braun                       address@hidden
 Matthew Clarke                      address@hidden
+Matthew Pfeiffer                    address@hidden
 Matthew M. Boedicker                address@hidden
 Matthew S. Levine                   address@hidden
 Matthew Smith                       address@hidden
diff --git a/src/rmdir.c b/src/rmdir.c
index c9f417957..a2f0f0813 100644
--- 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,8 @@ 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))));
 }
 
 /* Remove any empty parent directories of DIR.
@@ -133,18 +132,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 +233,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/tests/rmdir/ignore.sh b/tests/rmdir/ignore.sh
index d000c30cb..0d2be25ed 100755
--- a/tests/rmdir/ignore.sh
+++ b/tests/rmdir/ignore.sh
@@ -29,4 +29,16 @@ 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
+
 Exit $fail
-- 
2.24.1