On 25/09/2022 00:37, Paul Eggert wrote:
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.
This looks good.
How about the attached to add a NEWS entry,
and add DS_EMPTY, DS_NONEMPTY enums to make the code easier to read?
thanks,
Pádraig
diff --git a/NEWS b/NEWS
index d1c400e67..7f4eab921 100644
--- a/NEWS
+++ b/NEWS
@@ -60,6 +60,10 @@ GNU coreutils NEWS -*- outline -*-
date --debug now diagnoses if multiple --date or --set options are
specified, as only the last specified is significant in that case.
+ rm outputs more accurate diagnostics in the presence of errors
+ when removing directories. For example EIO will be faithfully
+ diagnosed, rather than being conflated with ENOTEMPTY.
+
* Noteworthy changes in release 9.1 (2022-04-15) [stable]
diff --git a/src/remove.c b/src/remove.c
index 0b6754bf7..a19794d5e 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -164,11 +164,11 @@ write_protected_non_symlink (int fd_cwd,
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. */
+ Use *DIR_STATUS as a cache for the status. */
static int
get_dir_status (FTS const *fts, FTSENT const *ent, int *dir_status)
{
- if (*dir_status < -1)
+ if (*dir_status == DS_UNKNOWN)
*dir_status = directory_status (fts->fts_cwd_fd, ent->fts_accpath);
return *dir_status;
}
@@ -254,7 +254,7 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
prompting the user */
if ( ! (x->recursive
|| (x->remove_empty_directories
- && get_dir_status (fts, ent, dir_status) < 0)))
+ && get_dir_status (fts, ent, dir_status) == DS_EMPTY)))
{
write_protected = -1;
wp_errno = *dir_status <= 0 ? EISDIR : *dir_status;
@@ -273,7 +273,7 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
/* Issue the prompt. */
if (dirent_type == DT_DIR
&& mode == PA_DESCEND_INTO_DIR
- && get_dir_status (fts, ent, dir_status) == 0)
+ && get_dir_status (fts, ent, dir_status) == DS_NONEMPTY)
fprintf (stderr,
(write_protected
? _("%s: descend into write-protected directory %s? ")
@@ -427,14 +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;
+ int dir_status = DS_UNKNOWN;
switch (ent->fts_info)
{
case FTS_D: /* preorder directory */
if (! x->recursive
&& !(x->remove_empty_directories
- && get_dir_status (fts, ent, &dir_status) < 0))
+ && get_dir_status (fts, ent, &dir_status) == DS_EMPTY))
{
/* This is the first (pre-order) encounter with a directory
that we cannot delete.
@@ -512,7 +512,7 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
enum RM_status s = prompt (fts, ent, true /*is_dir*/, x,
PA_DESCEND_INTO_DIR, &dir_status);
- if (s == RM_USER_ACCEPTED && dir_status == -1)
+ if (s == RM_USER_ACCEPTED && dir_status == DS_EMPTY)
{
/* When we know (from prompt when in interactive mode)
that this is an empty directory, don't prompt twice. */
diff --git a/src/system.h b/src/system.h
index 91228ec13..c63b66741 100644
--- a/src/system.h
+++ b/src/system.h
@@ -291,6 +291,11 @@ readdir_ignoring_dot_and_dotdot (DIR *dirp)
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. */
+enum {
+ DS_UNKNOWN = -2,
+ DS_EMPTY = -1,
+ DS_NONEMPTY = 0,
+};
static inline int
directory_status (int fd_cwd, char const *dir)
{
@@ -316,7 +321,7 @@ directory_status (int fd_cwd, char const *dir)
no_direntries = !readdir_ignoring_dot_and_dotdot (dirp);
saved_errno = errno;
closedir (dirp);
- return no_direntries && saved_errno == 0 ? -1 : saved_errno;
+ return no_direntries && saved_errno == 0 ? DS_EMPTY : saved_errno;
}
/* Factor out some of the common --help and --version processing code. */
