rm: two small bug fixes + new snapshot

[Jim Meyering]

I tried to remove a directory and got errors about
"Too many levels of symbolic links", while I expected a mere
"Permission denied".  The patches below were the result.

I've put new snapshots here:

    http://meyering.net/cu/coreutils-6.9-ss.tar.gz
    http://meyering.net/cu/coreutils-6.9-ss.tar.gz.sig

    http://meyering.net/cu/coreutils-6.9-281-a7ec.tar.gz
    http://meyering.net/cu/coreutils-6.9-281-a7ec.tar.gz.sig

For those of you prudent enough to verify signatures before
untarring, you'll see that I'm using a new signing subkey.
You can get an updated copy like this:

    gpg --keyserver subkeys.pgp.net --recv-keys B9AB9A16

2007-09-22  Jim Meyering  <address@hidden>

        rm: give a sensible diagnostic when failing to remove a symlink
        On some systems (those with openat et al), when rm would fail to
        remove a symlink, it would fail with the misleading diagnostic,
        "Too many levels of symbolic links".
        * NEWS: Mention the bug fix.
        * src/remove.c (is_nondir_lstat): New function.
        (remove_entry): Use it to catch failed-to-remove symlink (and any
        other non-dir) here so that we don't fall through and try to treat
        it as directory, which -- with a symlink -- would provoke the bogus
        ELOOP failure.
        * tests/rm/fail-eacces: Add a test for the above.
        * src/c99-to-c89.diff: Adjust offsets.

        rm: fix a tiny, nearly inconsequential bug.
        Don't perform a "."-relative lstat, when the file in question
        may well not be in ".".  Although this is a bug, a few attempts
        to exercise it on a linux-2.6.22 system failed.  You probably need
        a pre-openat system to trigger the failure.  The consequence of this
        bug would be a lower-quality diagnostic upon failed dir removal.
        * src/remove.c (is_dir_lstat): Add a parameter, fd_cwd.
        Use it instead of hard-coding AT_FDCWD.
        (remove_entry): Call is_dir_lstat with fd_cwd.

As usual, you can find full diffs here:

    http://git.sv.gnu.org/gitweb/?p=coreutils.git

But for convenience, here are the parts that affect remove.c:

diff --git a/src/remove.c b/src/remove.c
index 773ed12..aae7a88 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -927,11 +927,11 @@ prompt (int fd_cwd, Dirstack_state const *ds, char const 
*filename,
    *ST is FILENAME's tstatus.
    Do not modify errno.  */
 static inline bool
-is_dir_lstat (char const *filename, struct stat *st)
+is_dir_lstat (int fd_cwd, char const *filename, struct stat *st)
 {
   int saved_errno = errno;
   bool is_dir =
-    (cache_fstatat (AT_FDCWD, filename, st, AT_SYMLINK_NOFOLLOW) == 0
+    (cache_fstatat (fd_cwd, filename, st, AT_SYMLINK_NOFOLLOW) == 0
      && S_ISDIR (st->st_mode));
   errno = saved_errno;
   return is_dir;
@@ -1061,7 +1061,8 @@ remove_entry (int fd_cwd, Dirstack_state const *ds, char 
const *filename,
       /* Upon a failed attempt to unlink a directory, most non-Linux systems
         set errno to the POSIX-required value EPERM.  In that case, change
         errno to EISDIR so that we emit a better diagnostic.  */
-      if (! x->recursive && errno == EPERM && is_dir_lstat (filename, st))
+      if (! x->recursive && errno == EPERM && is_dir_lstat (fd_cwd,
+                                                           filename, st))
        errno = EISDIR;

       if (! x->recursive

diff --git a/src/remove.c b/src/remove.c
index aae7a88..ac10109 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -937,6 +937,21 @@ is_dir_lstat (int fd_cwd, char const *filename, struct 
stat *st)
   return is_dir;
 }

+/* Return true if FILENAME is a non-directory.
+   Otherwise, including the case in which lstat fails, return false.
+   *ST is FILENAME's tstatus.
+   Do not modify errno.  */
+static inline bool
+is_nondir_lstat (int fd_cwd, char const *filename, struct stat *st)
+{
+  int saved_errno = errno;
+  bool is_non_dir =
+    (cache_fstatat (fd_cwd, filename, st, AT_SYMLINK_NOFOLLOW) == 0
+     && !S_ISDIR (st->st_mode));
+  errno = saved_errno;
+  return is_non_dir;
+}
+
 #define DO_UNLINK(Fd_cwd, Filename, X)                                 \
   do                                                                   \
     {                                                                  \
@@ -1066,7 +1081,9 @@ remove_entry (int fd_cwd, Dirstack_state const *ds, char 
const *filename,
        errno = EISDIR;

       if (! x->recursive
-         || (cache_stat_ok (st) && !S_ISDIR (st->st_mode)))
+         || (cache_stat_ok (st) && !S_ISDIR (st->st_mode))
+         || (errno == EACCES && is_nondir_lstat (fd_cwd, filename, st))
+         )
        {
          if (ignorable_missing (x, errno))
            return RM_OK;