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.