From: Pádraig Brady Date: Fri, 31 Jan 2020 01:46:40 +0000 (-0500) Subject: rmdir: fix --ignore-fail-on-non-empty with permissions errors X-Git-Tag: v8.32~27 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1f443fe57291918ba245b2ffb7cdc07e64630af4;p=thirdparty%2Fcoreutils.git 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 5231b84ac3..8a349634e7 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 23b089754c..48f6c04c72 100644 --- 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 c9f417957e..7301db5ee3 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,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 9d777680c2..ebf68349ab 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 d000c30cbb..65e92d012c 100755 --- 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