bug#8154: du: issue with `--files0-from=DIR'

[Jim Meyering]

Stefan Vargyas wrote:

> Dear maintainers,
>
> While building and running coreutils v8.9, I came across the following
> issue of 'du':
>
>   $ mkdir /tmp/foo
>   $ du --files0-from=/tmp/foo
>   du: `/tmp/foo': read error: Is a directory
>   ...
>
> The program enters an infinite loop -- continuously printing on stderr
> the error message shown above. Although such usage pattern of 'du' is
> erroneous, it better not behave this way. Looking into 'du.c', I found
> that the unending loop is caused by a misconceived 'continue'
> statement placed after a call to 'error' (the one labeled by 'case
> AI_ERR_READ'). A plausible fixing patch is immediate: see it enclosed.
>
> Sincerely,
>
> Stefan Vargyas.
>
>
>
>
> --- coreutils-8.9/src/du.c    2011-01-01 23:19:23.000000000 +0200
> +++ coreutils-8.9-stev/src/du.c       2011-03-02 03:32:04.000000000 +0200
> @@ -926,8 +926,10 @@
>            switch (ai_err)
>              {
>              case AI_ERR_READ:
> -              error (0, errno, _("%s: read error"), quote (files_from));
> -              continue;
> +              error (EXIT_FAILURE, errno, _("%s: read error"),
> +                     quote (files_from));
> +              ok = false;
> +              goto out_argv_iter;
>
>              case AI_ERR_MEM:
>                xalloc_die ();
> @@ -978,6 +980,7 @@
>            ok &= du_files (temp_argv, bit_flags);
>          }
>      }
> +  out_argv_iter:

Thanks for the patch.
Contrary to what my alternative patch suggested, part of yours is required.
The part that exits the loop upon read error.
Though note that as you wrote it, the goto was unreachable,
since error (EXIT_FAILURE, ... never returns, while error (0, ...
merely issues a warning.

I've taken the opportunity (of this new label) to move the test
for EOF into the case alongside the other AI_ERR_* values:

diff --git a/src/du.c b/src/du.c
index 671cac7..6270092 100644
--- a/src/du.c
+++ b/src/du.c
@@ -889,6 +889,8 @@ main (int argc, char **argv)
                quote (files_from));

       ai = argv_iter_init_stream (stdin);
+      if (ai == NULL && errno == EISDIR)
+        error (EXIT_FAILURE, errno, _("invalid file: %s"), quote (files_from));

       /* It's not easy here to count the arguments, so assume the
          worst.  */
@@ -926,15 +928,17 @@ main (int argc, char **argv)
       bool skip_file = false;
       enum argv_iter_err ai_err;
       char *file_name = argv_iter (ai, &ai_err);
-      if (ai_err == AI_ERR_EOF)
-        break;
       if (!file_name)
         {
           switch (ai_err)
             {
+            case AI_ERR_EOF:
+              goto argv_iter_done;
+
             case AI_ERR_READ:
               error (0, errno, _("%s: read error"), quote (files_from));
-              continue;
+              ok = false;
+              goto argv_iter_done;

             case AI_ERR_MEM:
               xalloc_die ();
@@ -985,6 +989,7 @@ main (int argc, char **argv)
           ok &= du_files (temp_argv, bit_flags);
         }
     }
+ argv_iter_done:

   argv_iter_free (ai);
   di_set_free (di_set);