I ran into this problem when attempting to recursively
remove a directory in a filesystem on flaky hardware.
Although the underlying readdir syscall failed with errno == EIO,
rm issued no diagnostic about the I/O error.
Without this patch I see this behavior:
$ rm -fr baddir
rm: cannot remove 'baddir': Directory not empty
$ rm -ir baddir
rm: descend into directory 'baddir'? y
rm: remove directory 'baddir'? y
rm: cannot remove 'baddir': Directory not empty
With this patch I see the following behavior, which
lets the user know about the I/O error when rm tries
to read baddir's directory entries:
$ rm -fr baddir
rm: cannot remove 'baddir': Input/output error
$ rm -ir baddir
rm: cannot remove 'baddir': Input/output error
* src/remove.c (Ternary): Remove. All uses removed.
(get_dir_status): New static function.
(prompt): Last arg is now directory status, not ternary.
Return RM_USER_ACCEPTED if user explicitly accepted.
All uses changed.
Report any significant error in directory status right away.
(prompt, rm_fts): Use get_dir_status to get directory status lazily.
(excise): Treat any FTS_DNR errno as being more descriptive, not
just EPERM and EACCESS. For example, EIO is more descriptive.
(rm_fts): Distinguish more clearly between explicit and implied
user OK.
* src/remove.h (RM_USER_ACCEPTED): New constant.
(VALID_STATUS): Treat it as valid.
* src/system.h (is_empty_dir): Remove, replacing with ...
(directory_status): ... this more-general function.
All uses changed. Avoid undefined behavior of looking at
a non-null readdir pointer after corresponding closedir.
* tests/rm/rm-readdir-fail.sh: Adjust test of internals
to match current behavior.
---
src/remove.c | 80 +++++++++++++++++++------------------
src/remove.h | 4 +-
src/rmdir.c | 3 +-
src/system.h | 25 ++++++------
tests/rm/rm-readdir-fail.sh | 1 +
5 files changed, 58 insertions(+), 55 deletions(-)
diff --git a/src/remove.c b/src/remove.c
index 6756c409d..0b6754bf7 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -33,14 +33,6 @@
#include "xfts.h"
#include "yesno.h"
-enum Ternary
- {
- T_UNKNOWN = 2,
- T_NO,
- T_YES
- };
-typedef enum Ternary Ternary;
-
/* The prompt function may be called twice for a given directory.
The first time, we ask whether to descend into it, and the
second time, we ask whether to remove it. */
@@ -168,9 +160,23 @@ write_protected_non_symlink (int fd_cwd,
}
}
-/* Prompt whether to remove FILENAME (ent->, if required via a combination of
+/* Return the status of the directory identified by FTS and ENT.
+ This is -1 if the directory is empty, 0 if it is nonempty,
+ and a positive error number if there was trouble determining the status,
+ e.g., it is not a directory, or permissions problems, or I/O errors.
+ Use *DIR_STATUS is a cache for the status. */
+static int
+get_dir_status (FTS const *fts, FTSENT const *ent, int *dir_status)
+{
+ if (*dir_status < -1)
+ *dir_status = directory_status (fts->fts_cwd_fd, ent->fts_accpath);
+ return *dir_status;
+}
+
+/* Prompt whether to remove FILENAME, if required via a combination of
the options specified by X and/or file attributes. If the file may
- be removed, return RM_OK. If the user declines to remove the file,
+ be removed, return RM_OK or RM_USER_ACCEPTED, the latter if the user
+ was prompted and accepted. If the user declines to remove the file,
return RM_USER_DECLINED. If not ignoring missing files and we
cannot lstat FILENAME, then return RM_ERROR.
@@ -178,20 +184,16 @@ write_protected_non_symlink (int fd_cwd,
Depending on MODE, ask whether to 'descend into' or to 'remove' the
directory FILENAME. MODE is ignored when FILENAME is not a directory.
- Set *IS_EMPTY_P to T_YES if FILENAME is an empty directory, and it is
- appropriate to try to remove it with rmdir (e.g. recursive mode).
- Don't even try to set *IS_EMPTY_P when MODE == PA_REMOVE_DIR. */
+ Use and update *DIR_STATUS as needed, via the conventions of
+ get_dir_status. */
static enum RM_status
prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
struct rm_options const *x, enum Prompt_action mode,
- Ternary *is_empty_p)
+ int *dir_status)
{
int fd_cwd = fts->fts_cwd_fd;
char const *full_name = ent->fts_path;
char const *filename = ent->fts_accpath;
- if (is_empty_p)
- *is_empty_p = T_UNKNOWN;
-
struct stat st;
struct stat *sbuf = &st;
cache_stat_init (sbuf);
@@ -199,13 +201,6 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
int dirent_type = is_dir ? DT_DIR : DT_UNKNOWN;
int write_protected = 0;
- bool is_empty = false;
- if (is_empty_p)
- {
- is_empty = is_empty_dir (fd_cwd, filename);
- *is_empty_p = is_empty ? T_YES : T_NO;
- }
-
/* When nonzero, this indicates that we failed to remove a child entry,
either because the user declined an interactive prompt, or due to
some other failure, like permissions. */
@@ -257,10 +252,12 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
/* Unless we're either deleting directories or deleting
recursively, we want to raise an EISDIR error rather than
prompting the user */
- if ( ! (x->recursive || (x->remove_empty_directories && is_empty)))
+ if ( ! (x->recursive
+ || (x->remove_empty_directories
+ && get_dir_status (fts, ent, dir_status) < 0)))
{
write_protected = -1;
- wp_errno = EISDIR;
+ wp_errno = *dir_status <= 0 ? EISDIR : *dir_status;
}
break;
}
@@ -276,12 +273,17 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
/* Issue the prompt. */
if (dirent_type == DT_DIR
&& mode == PA_DESCEND_INTO_DIR
- && !is_empty)
+ && get_dir_status (fts, ent, dir_status) == 0)
fprintf (stderr,
(write_protected
? _("%s: descend into write-protected directory %s? ")
: _("%s: descend into directory %s? ")),
program_name, quoted_name);
+ else if (0 < *dir_status)
+ {
+ error (0, *dir_status, _("cannot remove %s"), quoted_name);
+ return RM_ERROR;
+ }
else
{
if (cache_fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW) != 0)
@@ -302,8 +304,7 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
program_name, file_type (sbuf), quoted_name);
}
- if (!yesno ())
- return RM_USER_DECLINED;
+ return yesno () ? RM_USER_ACCEPTED : RM_USER_DECLINED;
}
return RM_OK;
}
@@ -405,13 +406,12 @@ excise (FTS *fts, FTSENT *ent, struct rm_options const
*x, bool is_dir)
/* When failing to rmdir an unreadable directory, we see errno values
like EISDIR or ENOTDIR (or, on Solaris 10, EEXIST), but they 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
+ meaningless in a diagnostic. When that happens, use the earlier, more
descriptive errno value. */
if (ent->fts_info == FTS_DNR
&& (errno == ENOTEMPTY || errno == EISDIR || errno == ENOTDIR
|| errno == EEXIST)
- && (ent->fts_errno == EPERM || ent->fts_errno == EACCES))
+ && ent->fts_errno != 0)
errno = ent->fts_errno;
error (0, errno, _("cannot remove %s"), quoteaf (ent->fts_path));
mark_ancestor_dirs (ent);
@@ -427,12 +427,14 @@ excise (FTS *fts, FTSENT *ent, struct rm_options const
*x, bool is_dir)
static enum RM_status
rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
{
+ int dir_status = -2;
+
switch (ent->fts_info)
{
case FTS_D: /* preorder directory */
if (! x->recursive
&& !(x->remove_empty_directories
- && is_empty_dir (fts->fts_cwd_fd, ent->fts_accpath)))
+ && get_dir_status (fts, ent, &dir_status) < 0))
{
/* This is the first (pre-order) encounter with a directory
that we cannot delete.
@@ -507,11 +509,10 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
}
{
- Ternary is_empty_directory;
enum RM_status s = prompt (fts, ent, true /*is_dir*/, x,
- PA_DESCEND_INTO_DIR, &is_empty_directory);
+ PA_DESCEND_INTO_DIR, &dir_status);
- if (s == RM_OK && is_empty_directory == T_YES)
+ if (s == RM_USER_ACCEPTED && dir_status == -1)
{
/* When we know (from prompt when in interactive mode)
that this is an empty directory, don't prompt twice. */
@@ -520,7 +521,7 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
fts_skip_tree (fts, ent);
}
- if (s != RM_OK)
+ if (! (s == RM_OK || s == RM_USER_ACCEPTED))
{
mark_ancestor_dirs (ent);
fts_skip_tree (fts, ent);
@@ -553,8 +554,9 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
}
bool is_dir = ent->fts_info == FTS_DP || ent->fts_info == FTS_DNR;
- enum RM_status s = prompt (fts, ent, is_dir, x, PA_REMOVE_DIR, NULL);
- if (s != RM_OK)
+ enum RM_status s = prompt (fts, ent, is_dir, x, PA_REMOVE_DIR,
+ &dir_status);
+ if (! (s == RM_OK || s == RM_USER_ACCEPTED))
return s;
return excise (fts, ent, x, is_dir);
}
diff --git a/src/remove.h b/src/remove.h
index e926084e2..b92c63daa 100644
--- a/src/remove.h
+++ b/src/remove.h
@@ -79,13 +79,15 @@ enum RM_status
{
/* These must be listed in order of increasing seriousness. */
RM_OK = 2,
+ RM_USER_ACCEPTED,
RM_USER_DECLINED,
RM_ERROR,
RM_NONEMPTY_DIR
};
# define VALID_STATUS(S) \
- ((S) == RM_OK || (S) == RM_USER_DECLINED || (S) == RM_ERROR)
+ ((S) == RM_OK || (S) == RM_USER_ACCEPTED || (S) == RM_USER_DECLINED \
+ || (S) == RM_ERROR)
# define UPDATE_STATUS(S, New_value) \
do \
diff --git a/src/rmdir.c b/src/rmdir.c
index f6284cbe2..283f332ec 100644
--- a/src/rmdir.c
+++ b/src/rmdir.c
@@ -101,8 +101,7 @@ ignorable_failure (int error_number, char const *dir)
return (ignore_fail_on_non_empty
&& (errno_rmdir_non_empty (error_number)
|| (errno_may_be_non_empty (error_number)
- && ! is_empty_dir (AT_FDCWD, dir)
- && errno == 0 /* definitely non empty */)));
+ && directory_status (AT_FDCWD, dir) == 0)));
}
/* Remove any empty parent directories of DIR.
diff --git a/src/system.h b/src/system.h
index d36ca1115..91228ec13 100644
--- a/src/system.h
+++ b/src/system.h
@@ -287,37 +287,36 @@ readdir_ignoring_dot_and_dotdot (DIR *dirp)
}
}
-/* Return true if DIR is determined to be an empty directory.
- Return false with ERRNO==0 if DIR is a non empty directory.
- Return false if not able to determine if directory empty. */
-static inline bool
-is_empty_dir (int fd_cwd, char const *dir)
+/* Return -1 if DIR is an empty directory,
+ 0 if DIR is a nonempty directory,
+ and a positive error number if there was trouble determining
+ whether DIR is an empty or nonempty directory. */
+static inline int
+directory_status (int fd_cwd, char const *dir)
{
DIR *dirp;
- struct dirent const *dp;
+ bool no_direntries;
int saved_errno;
int fd = openat (fd_cwd, dir,
(O_RDONLY | O_DIRECTORY
| O_NOCTTY | O_NOFOLLOW | O_NONBLOCK));
if (fd < 0)
- return false;
+ return errno;
dirp = fdopendir (fd);
if (dirp == NULL)
{
+ saved_errno = errno;
close (fd);
- return false;
+ return saved_errno;
}
errno = 0;
- dp = readdir_ignoring_dot_and_dotdot (dirp);
+ no_direntries = !readdir_ignoring_dot_and_dotdot (dirp);
saved_errno = errno;
closedir (dirp);
- errno = saved_errno;
- if (dp != NULL)
- return false;
- return saved_errno == 0 ? true : false;
+ return no_direntries && saved_errno == 0 ? -1 : saved_errno;
}
/* Factor out some of the common --help and --version processing code. */
diff --git a/tests/rm/rm-readdir-fail.sh b/tests/rm/rm-readdir-fail.sh
index a77d5225f..9dbf9380c 100755
--- a/tests/rm/rm-readdir-fail.sh
+++ b/tests/rm/rm-readdir-fail.sh
@@ -112,6 +112,7 @@ done
# (with ENOENT in this case but it could be anything).
cat <<EOF > exp
rm: cannot remove 'dir'
+Failed to get dirent
rm: traversal failed: dir
EOF
sed 's/\(rm:.*\):.*/\1/' errt > err || framework_failure_
--
2.37.3
