Re: areadlinkat

[Jim Meyering]

Eric Blake wrote:
> According to Eric Blake on 10/21/2009 9:33 PM:
>>> +   * lib/at-func.c (FUNC_FAIL): New define.
>>> +   (AT_FUNC_NAME, VALIDATE_FLAG): Use it rather than raw -1.
>>
>> Oops - I missed one instance relating to FUNC_FAIL.  test-areadlinkat was
>> failing on older Linux, because /proc/self/fd/-1/name gave ENOENT rather
>> than EBADF.  But since 0 <= char* was always true, the fallback code that
>> converts ENOENT into the correct error was never triggered.
>> areadlinkat-with-size was the only other impacted client of at-func.c.
>
> And yet one more instance, this time caught by gcc warnings, but harder to
> trigger in code.
...
> +     areadlinkat: fix fallback path
> +     * lib/at-func.c (AT_FUNC_NAME): Avoid signed comparison between
> +     pointer and zero.
...
>    err = CALL_FUNC (file);
> -  saved_errno = (err < 0 ? errno : 0);
> +  saved_errno = (err == FUNC_FAIL ? errno : 0);

Nice.
It's gratifying when the work of complying with newly-enabled
warnings pays off.

This made me think it will be worthwhile to use a separate set of
gcc warning options for use with -Werror on a given package's
gnulib-selected lib/* files.

As a result, in coreutils I've pruned a few warning options from
our primary list and added a handful of gl/lib/*.diff patches to
make it so building in lib/ now passes with -Werror.

Patch coming up as soon as I tweak the syntax-check rules
to permit the new *.diff files.