Subject: bug#58050: [INSTALLED] rm: fix diagnostics on I/O error

From: Pádraig Brady <[email-masked]>
Date: Sun, 25 Sep 2022 15:25:25 +0100

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.  */