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

[Jim Meyering]

Jim Meyering wrote:
> Jim Meyering wrote:
>> Paul Eggert wrote:
>>> On 03/02/2011 07:09 AM, Jim Meyering wrote:
>>>> -  struct argv_iterator *ai = malloc (sizeof *ai);
>>>> +  struct argv_iterator *ai;
>>>> +  struct stat st;
>>>> +
>>>> +  if (fstat (fileno (fp),&st) == 0&&  S_ISDIR (st.st_mode))
>>>> +    {
>>>> +      errno = EISDIR;
>>>> +      return NULL;
>>>> +    }
>>>> +
>>>> +  ai = malloc (sizeof *ai);
>>>
>>> My kneejerk reaction is that this part of the patch
>>> should not be needed (though other fixes obviously are).
>>> There are lots of reasons that the stream could fail:
>>> why check for directories specially?  Just let the
>>> stream fail in the way that it normally would; this
>>> keeps the code smaller and simpler.  On an ancient
>>> host where "cat dir/" works, "du --files0-from=dir/"
>>> should not arbitrarily fail.
>>
>> I think you're right.  The added complexity (both here
>> and in each client) is not worth the theoretical gain
>> in error-handling uniformity.
>>
>> If new tests induce false-positive failure due
>> to differences in how reading from a directory works
>> under the covers, we can deal with it in the tests.
>
> If we decide it's worthwhile, we can adjust gnulib later.
> For now, I'm about to fix coreutils with these two patches:
>
> Subject: [PATCH 1/2] wc: avoid NULL dereference on out-of-memory error

I've pushed that, with adjusted logs:
(mainly, I dug up and noted each bug-introducing commit)

commit caaf2899f67d312d76af91add2a4d9f7be2d5c61
Author: Jim Meyering <address@hidden>
Date:   Wed Mar 2 19:16:46 2011 +0100

    du: don't infloop for --files0-from=DIR

    * src/du.c (main): Fail on AI_ERR_READ error, rather than merely
    diagnosing and continuing.  Based on a patch by Stefan Vargyas.
    Also move the handling of AI_ERR_EOF into the case stmt.
    Do not report ferror/fclose(stdin) failure when we've
    already diagnosed e.g., failure to read the DIR, above.
    Bug introduced by 2008-11-24 commit 031e2fb5, "du: read and
    process --files0-from= input a name at a time,".
    * src/wc.c: Handle read failure as with du: do not exit
    immediately, but rather go on to print any total and to clean-up.
    As above, move the handling of AI_ERR_EOF into the case stmt.
    * tests/du/files0-from-dir: New file, to test both du and wc.
    * tests/Makefile.am (TESTS): Add it.
    * NEWS (Bug fixes): Mention it.

commit 7cfd12c78e0be4c90f29c99ab383163aa1471504
Author: Jim Meyering <address@hidden>
Date:   Wed Mar 2 18:54:43 2011 +0100

    wc: avoid NULL dereference on out-of-memory error

    * src/wc.c (main): Diagnose failed argv_iter_init_* failure,
    rather than falling through and dereferencing NULL.
    Bug introduced by 2008-11-25 commit c2e56e0d,
    "wc: read and process --files0-from= input a name at a time,".
    * NEWS (Bug fixes): Mention it.