bug#39364: [PATCH] rmdir: fix clobbered errno

[Pádraig Brady]

On 03/02/2020 13:26, Pádraig Brady wrote:
> On 31/01/2020 17:51, Pádraig Brady wrote:
> Actually I think the key issue is not errno handling,
> but a logic error fixed with:
>
> @@ -102,7 +102,7 @@ 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))));
> +                  && ! is_empty_dir (AT_FDCWD, dir))));
>
>
> Attached is a full patch to address these issues.

I'll also squash this in to the previous commit,
to ensure we diagnose the case where we can't
determine if the directory is empty.

diff --git a/src/rmdir.c b/src/rmdir.c
index a2f0f0813..7301db5ee 100644
--- a/src/rmdir.c
+++ b/src/rmdir.c
@@ -101,7 +101,8 @@ ignorable_failure (int error_number, char const *dir)
   return (ignore_fail_on_non_empty
           && (errno_rmdir_non_empty (error_number)
               || (errno_may_be_non_empty (error_number)
-                  && ! is_empty_dir (AT_FDCWD, dir))));
+                  && ! is_empty_dir (AT_FDCWD, dir)
+                  && errno == 0 /* definitely non empty  */)));
 }

 /* Remove any empty parent directories of DIR.
diff --git a/src/system.h b/src/system.h
index 9d777680c..ebf68349a 100644
--- 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 0d2be25ed..65e92d012 100755
--- a/tests/rmdir/ignore.sh
+++ b/tests/rmdir/ignore.sh
@@ -40,5 +40,10 @@ test -d x/y || fail=1
 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