[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: rm -r sometimes produces errors under NFS
From: |
Paul Eggert |
Subject: |
Re: rm -r sometimes produces errors under NFS |
Date: |
Thu, 08 Mar 2007 00:13:48 -0800 |
User-agent: |
Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux) |
Regardless of the NFS issue, if euidaccess discovers trouble with a
directory entry (i.e., euidaccess fails for reasons other than
EACCES), then it would make sense for 'rm' to issue a diagnostic
rather than ignoring the problem and going ahead with 'unlink'.
For Vincent's case, this would cause 'rm' to issue a diagnostic like
"rm: cannot remove `dir/file:': Stale NFS file handle" which is more
accurate than the somewhat confusing "no such file or directory"
diagnostic that 'rm' currently generates.
Here's a more-diabolical example. You invoke 'rm a/b/c', and a/b/c is
read-only. Some other process temporarily renames a/b to a/B between
the time that 'rm' stats a/b/c and the time 'rm' invokes
euidaccess("a/b/c", W_OK); the other process then renames a/B back to
a/b after euidaccess is invoked. In this case, euidaccess fails with
ENOENT but the unpatched 'rm' will go ahead and remove a/b/c (even
though a/b/c is read-only!) whereas the patched 'rm' will diagnose the
problem and refuse to remove a/b/c.
Admittedly a contrived case, but hey! it's late.
Here's a proposed patch. Many of the changes are due to reindenting.
2007-03-07 Paul Eggert <address@hidden>
* src/remove.c (write_protected_non_symlink): Return int, not bool,
so that we can indicate failure too (as a postive error number).
(prompt): If write_protected_non_symlink fails, report that error
number and fail rather than charging ahead and removing the
dubious entry. Redo the logic of printing a diagnostic so that
we need to invoke quote (full_filename (...)) only once.
diff --git a/src/remove.c b/src/remove.c
index 97184eb..6f663ec 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -704,20 +704,21 @@ is_empty_dir (int fd_cwd, char const *dir)
return saved_errno == 0 ? true : false;
}
-/* Return true if FILE is determined to be an unwritable non-symlink.
- Otherwise, return false (including when lstat'ing it fails).
+/* Return -1 if FILE is an unwritable non-symlink,
+ 0 if it is some other kind of file,
+ a positive error number if there is some problem in determining the answer.
Set *BUF to the file status.
This is to avoid calling euidaccess when FILE is a symlink. */
-static bool
+static int
write_protected_non_symlink (int fd_cwd,
char const *file,
Dirstack_state const *ds,
struct stat *buf)
{
if (cache_fstatat (fd_cwd, file, buf, AT_SYMLINK_NOFOLLOW) != 0)
- return false;
+ return errno;
if (S_ISLNK (buf->st_mode))
- return false;
+ return 0;
/* Here, we know FILE is not a symbolic link. */
/* In order to be reentrant -- i.e., to avoid changing the working
@@ -771,9 +772,16 @@ write_protected_non_symlink (int fd_cwd,
size_t file_name_len
= obstack_object_size (&ds->dir_stack) + strlen (file);
- return (file_name_len < MIN (PATH_MAX, 8192)
- ? euidaccess (full_filename (file), W_OK) != 0 && errno == EACCES
- : euidaccess_stat (buf, W_OK) != 0);
+ if (MIN (PATH_MAX, 8192) <= file_name_len)
+ return - euidaccess_stat (buf, W_OK);
+ if (euidaccess (full_filename (file), W_OK) == 0)
+ return 0;
+ if (errno == EACCES)
+ return -1;
+
+ /* Perhaps some other process has removed the file, or perhaps this
+ is a buggy NFS client. */
+ return errno;
}
}
@@ -794,75 +802,75 @@ prompt (int fd_cwd, Dirstack_state const *ds, char const
*filename,
struct rm_options const *x, enum Prompt_action mode,
Ternary *is_empty)
{
- bool write_protected = false;
+ int write_protected = 0;
*is_empty = T_UNKNOWN;
if (x->interactive == RMI_NEVER)
return RM_OK;
- if (((!x->ignore_missing_files & ((x->interactive == RMI_ALWAYS)
- | x->stdin_tty))
- && (write_protected = write_protected_non_symlink (fd_cwd, filename,
- ds, sbuf)))
- || x->interactive == RMI_ALWAYS)
+ if (!x->ignore_missing_files
+ & ((x->interactive == RMI_ALWAYS) | x->stdin_tty))
+ write_protected = write_protected_non_symlink (fd_cwd, filename, ds, sbuf);
+
+ if (write_protected || x->interactive == RMI_ALWAYS)
{
- if (cache_fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW) != 0)
+ char const *quoted_name;
+
+ if (write_protected <= 0
+ && cache_fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW) != 0)
{
/* This happens, e.g., with `rm '''. */
- error (0, errno, _("cannot remove %s"),
- quote (full_filename (filename)));
- return RM_ERROR;
+ write_protected = errno;
}
- if (S_ISDIR (sbuf->st_mode) && !x->recursive)
+ if (write_protected <= 0)
{
- error (0, EISDIR, _("cannot remove %s"),
- quote (full_filename (filename)));
- return RM_ERROR;
+ /* Using permissions doesn't make sense for symlinks. */
+ if (S_ISLNK (sbuf->st_mode) && x->interactive != RMI_ALWAYS)
+ return RM_OK;
+
+ if (S_ISDIR (sbuf->st_mode) && !x->recursive)
+ write_protected = EISDIR;
}
- /* Using permissions doesn't make sense for symlinks. */
- if (S_ISLNK (sbuf->st_mode))
+ quoted_name = quote (full_filename (filename));
+
+ if (0 < write_protected)
{
- if (x->interactive != RMI_ALWAYS)
- return RM_OK;
- write_protected = false;
+ error (0, write_protected, _("cannot remove %s"), quoted_name);
+ return RM_ERROR;
}
/* Issue the prompt. */
- {
- char const *quoted_name = quote (full_filename (filename));
-
- /* FIXME: use a variant of error (instead of fprintf) that doesn't
- append a newline. Then we won't have to declare program_name in
- this file. */
- if (S_ISDIR (sbuf->st_mode)
- && x->recursive
- && mode == PA_DESCEND_INTO_DIR
- && ((*is_empty = (is_empty_dir (fd_cwd, filename) ? T_YES : T_NO))
- == T_NO))
+ /* FIXME: use a variant of error (instead of fprintf) that doesn't
+ append a newline. Then we won't have to declare program_name in
+ this file. */
+ if (S_ISDIR (sbuf->st_mode)
+ && x->recursive
+ && mode == PA_DESCEND_INTO_DIR
+ && ((*is_empty = (is_empty_dir (fd_cwd, filename) ? T_YES : T_NO))
+ == T_NO))
+ fprintf (stderr,
+ (write_protected
+ ? _("%s: descend into write-protected directory %s? ")
+ : _("%s: descend into directory %s? ")),
+ program_name, quoted_name);
+ else
+ {
+ /* TRANSLATORS: You may find it more convenient to translate
+ the equivalent of _("%s: remove %s (write-protected) %s? ").
+ It should avoid grammatical problems with the output
+ of file_type. */
fprintf (stderr,
(write_protected
- ? _("%s: descend into write-protected directory %s? ")
- : _("%s: descend into directory %s? ")),
- program_name, quoted_name);
- else
- {
- /* TRANSLATORS: You may find it more convenient to translate
- the equivalent of _("%s: remove %s (write-protected) %s? ").
- It should avoid grammatical problems with the output
- of file_type. */
- fprintf (stderr,
- (write_protected
- ? _("%s: remove write-protected %s %s? ")
- : _("%s: remove %s %s? ")),
- program_name, file_type (sbuf), quoted_name);
- }
+ ? _("%s: remove write-protected %s %s? ")
+ : _("%s: remove %s %s? ")),
+ program_name, file_type (sbuf), quoted_name);
+ }
- if (!yesno ())
- return RM_USER_DECLINED;
- }
+ if (!yesno ())
+ return RM_USER_DECLINED;
}
return RM_OK;
}
M ChangeLog
M src/remove.c
Committed as f503a8dd4f25abb2e54b9a332e0caebb1cce49f4
- Re: rm -r sometimes produces errors under NFS, (continued)
- Re: rm -r sometimes produces errors under NFS, Vincent Lefevre, 2007/03/06
- Re: rm -r sometimes produces errors under NFS, Jim Meyering, 2007/03/07
- Re: rm -r sometimes produces errors under NFS, Vincent Lefevre, 2007/03/08
- Re: rm -r sometimes produces errors under NFS, Jim Meyering, 2007/03/08
- Re: rm -r sometimes produces errors under NFS, Vincent Lefevre, 2007/03/08
- Re: rm -r sometimes produces errors under NFS, Jim Meyering, 2007/03/10
- Re: rm -r sometimes produces errors under NFS, Vincent Lefevre, 2007/03/12
- Re: rm -r sometimes produces errors under NFS, Paul Eggert, 2007/03/06
- Re: rm -r sometimes produces errors under NFS, Jim Meyering, 2007/03/07
- Re: rm -r sometimes produces errors under NFS, Paul Eggert, 2007/03/07
- Re: rm -r sometimes produces errors under NFS,
Paul Eggert <=
- Re: rm -r sometimes produces errors under NFS, Vincent Lefevre, 2007/03/08
- Re: rm -r sometimes produces errors under NFS, Jim Meyering, 2007/03/08
- Re: rm -r sometimes produces errors under NFS, James Youngman, 2007/03/05
- Re: rm -r sometimes produces errors under NFS, Linda Walsh, 2007/03/15
- Re: rm -r sometimes produces errors under NFS, Vincent Lefevre, 2007/03/15
- Re: rm -r sometimes produces errors under NFS, Paul Eggert, 2007/03/16
- Re: rm -r sometimes produces errors under NFS, Jim Meyering, 2007/03/16