diff options
| -rw-r--r-- | ChangeLog | 13 | ||||
| -rw-r--r-- | NEWS | 4 | ||||
| -rw-r--r-- | src/c99-to-c89.diff | 6 | ||||
| -rw-r--r-- | src/remove.c | 19 | ||||
| -rwxr-xr-x | tests/rm/fail-eacces | 23 |
5 files changed, 60 insertions, 5 deletions
@@ -1,5 +1,18 @@ 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 @@ -202,6 +202,10 @@ GNU coreutils NEWS -*- outline -*- "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 diff --git a/src/c99-to-c89.diff b/src/c99-to-c89.diff index 3e66bc480..01e213beb 100644 --- a/src/c99-to-c89.diff +++ b/src/c99-to-c89.diff @@ -42,7 +42,7 @@ diff -upr src/remove.c src/remove.c 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; } @@ -50,7 +50,7 @@ diff -upr src/remove.c src/remove.c 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 (); @@ -58,7 +58,7 @@ diff -upr src/remove.c src/remove.c 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; } diff --git a/src/remove.c b/src/remove.c index aae7a8888..ac1010953 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; diff --git a/tests/rm/fail-eacces b/tests/rm/fail-eacces index b15c2f544..d49764c53 100755 --- a/tests/rm/fail-eacces +++ b/tests/rm/fail-eacces @@ -1,5 +1,8 @@ #!/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. @@ -25,7 +28,19 @@ fi . $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 @@ -33,7 +48,13 @@ rm -rf d/f 2> out && fail=1 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 |