2007-09-22 Jim Meyering <jim@meyering.net>
+ 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
"rm --interactive=never F" no longer prompts for an unwritable F
+ "rm -rf D" would emit an misleading diagnostic when failing to
+ remove a symbolic link within the unwritable directory, D.
+ Introduced in coreutils-6.0.
+
** New features
sort's new --compress-program=PROG option specifies a compression
if (!yesno ())
return RM_USER_DECLINED;
-@@ -1515,6 +1519,7 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1533,6 +1537,7 @@ rm_1 (Dirstack_state *ds, char const *fi
return RM_ERROR;
}
struct stat st;
cache_stat_init (&st);
cycle_check_init (&ds->cycle_check_state);
-@@ -1537,6 +1542,7 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1555,6 +1560,7 @@ rm_1 (Dirstack_state *ds, char const *fi
AD_push_initial (ds);
AD_INIT_OTHER_MEMBERS ();
enum RM_status status = remove_entry (AT_FDCWD, ds, filename,
DT_UNKNOWN, &st, x);
if (status == RM_NONEMPTY_DIR)
-@@ -1555,6 +1561,8 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1573,6 +1579,8 @@ rm_1 (Dirstack_state *ds, char const *fi
ds_clear (ds);
return status;
}
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 \
{ \
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;
#!/bin/sh
# Ensure that rm -rf unremovable-non-dir gives a diagnostic.
+# Test both a regular file and a symlink -- it makes a difference to rm.
+# With the symlink, rm from coreutils-6.9 would fail with a misleading
+# ELOOP diagnostic.
# Copyright (C) 2006-2007 Free Software Foundation, Inc.
. $srcdir/../test-lib.sh
skip_if_root_
-mkdir d && touch d/f && chmod a-w d || framework_failure
+ok=0
+mkdir d &&
+ touch d/f &&
+ ln -s f d/slink &&
+ chmod a-w d &&
+ ok=1
+test $ok = 1 || framework_failure
+
+mkdir e &&
+ ln -s f e/slink &&
+ chmod a-w e &&
+ ok=1
+test $ok = 1 || framework_failure
fail=0
cat <<\EOF > exp
rm: cannot remove `d/f': Permission denied
EOF
+compare out exp || fail=1
+# This used to fail with ELOOP.
+rm -rf e 2> out && fail=1
+cat <<\EOF > exp
+rm: cannot remove `e/slink': Permission denied
+EOF
compare out exp || fail=1
(exit $fail); exit $fail