bug#12339: Bug: rm -fr . doesn't dir depth first deletion yet it is documented to do so.

[Jim Meyering]

Jim Meyering wrote:
...
> I'll post this new pair of rm-related patches in a minute.

Here's what I'm ready to push.
Finally, I'm not adding a test for what happens
when one tries to run rm -r s/ (with s being a symlink),
since that is so system-specific.  I suppose I could add
test solely to ensure that if it fails, it does not fail
with the ELOOP message, but that seems way too weak.


>From 57dd06703cb89ba53d05af95c11e89a2ca51af5c Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 4 Sep 2012 14:40:11 +0200
Subject: [PATCH 1/2] rm: avoid bogus diagnostic for a slash-decorated
 symlink-to-dir

These commands would evoke an invalid diagnostic:
    $ mkdir d && ln -s d s && env rm -r s/
    rm: cannot remove 's': Too many levels of symbolic links
remove.c was stripping trailing slashes from "s/" before passing
the name to "rm".  But a trailing slash may change the semantics,
and thus should not be stripped.
* src/remove.c (rm_fts): Do not strip trailing slashes.
* tests/rm/v-slash.sh: Adapt to new expected output.
* gnulib: Update to latest, for an improved fts.c that merely
normalizes trailing slashes.
Reported by Paul Eggert in discussion of http://bugs.gnu.org/12339
---
 NEWS                | 4 ++++
 gnulib              | 2 +-
 src/remove.c        | 3 ---
 tests/rm/v-slash.sh | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index f3874fd..a848ffe 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   than ignoring the -d option and failing with an 'Is a directory' error.
   [bug introduced in coreutils-8.19, with the addition of --dir (-d)]

+  rm -r S/ (where S is a symlink-to-directory) no longer gives the invalid
+  "Too many levels of symbolic links" diagnostic.
+  [bug introduced in coreutils-8.6]
+
 ** Improvements

   stat and tail work better with ZFS.  stat -f --format=%T now reports the
diff --git a/gnulib b/gnulib
index 68f693f..3a9002d 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 68f693ff1db33bf24695f0f42c62e7801966fd06
+Subproject commit 3a9002d3cc63da7110f133b1040d2d2b0aad8305
diff --git a/src/remove.c b/src/remove.c
index 69faae6..847a5cc 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -433,9 +433,6 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
       /* Perform checks that can apply only for command-line arguments.  */
       if (ent->fts_level == FTS_ROOTLEVEL)
         {
-          if (strip_trailing_slashes (ent->fts_path))
-            ent->fts_pathlen = strlen (ent->fts_path);
-
           /* If the basename of a command line argument is "." or "..",
              diagnose it and do nothing more with that argument.  */
           if (dot_or_dotdot (last_component (ent->fts_accpath)))
diff --git a/tests/rm/v-slash.sh b/tests/rm/v-slash.sh
index 504f4ff..ec77bd0 100755
--- a/tests/rm/v-slash.sh
+++ b/tests/rm/v-slash.sh
@@ -26,7 +26,7 @@ touch a/x || framework_failure_
 rm --verbose -r a/// > out || fail=1
 cat <<\EOF > exp || fail=1
 removed 'a/x'
-removed directory: 'a'
+removed directory: 'a/'
 EOF

 compare exp out || fail=1
--
1.7.12.176.g3fc0e4c


>From eda8c84778bc72192592b79724466946ca7def97 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 5 Sep 2012 16:48:50 +0200
Subject: [PATCH 2/2] rm: be more careful when using a replacement errno value

* src/remove.c (excise): Tighten the test for when we defer to an
old errno value: instead of relying solely on an FTS_DNR (unreadable
directory) failure, also test current and replacement errno values.
This change would also have solved the problem addressed by commit
v8.19-106-g57dd067.  For more info, see http://bugs.gnu.org/12339#113
---
 gnulib       |  2 +-
 src/remove.c | 12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/gnulib b/gnulib
index 3a9002d..68f693f 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 3a9002d3cc63da7110f133b1040d2d2b0aad8305
+Subproject commit 68f693ff1db33bf24695f0f42c62e7801966fd06
diff --git a/src/remove.c b/src/remove.c
index 847a5cc..0c25462 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -392,11 +392,13 @@ excise (FTS *fts, FTSENT *ent, struct rm_options const 
*x, bool is_dir)
   if (ignorable_missing (x, errno))
     return RM_OK;

-  /* When failing to rmdir an unreadable directory, the typical
-     errno value is EISDIR, but that is not as useful to the user
-     as the errno value from the failed open (probably EPERM).
-     Use the earlier, more descriptive errno value.  */
-  if (ent->fts_info == FTS_DNR)
+  /* When failing to rmdir an unreadable directory, the typical errno value
+     is EISDIR or ENOTDIR, but that would be meaningless in a diagnostic.
+     When that happens and the errno value from the failed open is EPERM
+     or EACCES, use the earlier, more descriptive errno value.  */
+  if (ent->fts_info == FTS_DNR
+      && (errno == ENOTEMPTY || errno == EISDIR || errno == ENOTDIR)
+      && (ent->fts_errno == EPERM || ent->fts_errno == EACCES))
     errno = ent->fts_errno;
   error (0, errno, _("cannot remove %s"), quote (ent->fts_path));
   mark_ancestor_dirs (ent);
--
1.7.12.176.g3fc0e4c