build warnings

build warnings

[Michael Wookey]

FYI,

With current master (dace5dd1), the following build warnings appear on
Ubuntu 9.10 (x86):

  run-command.c: In function ‘notify_parent’:
  run-command.c:70: warning: ignoring return value of ‘write’,
declared with attribute warn_unused_result
  run-command.c: In function ‘die_child’:
  run-command.c:80: warning: ignoring return value of ‘write’,
declared with attribute warn_unused_result
  run-command.c:81: warning: ignoring return value of ‘write’,
declared with attribute warn_unused_result
  run-command.c:82: warning: ignoring return value of ‘write’,
declared with attribute warn_unused_result

These warnings were introduced by the following commits:

  2b541bf8 ("start_command: detect execvp failures early")
  a5487ddf ("start_command: report child process setup errors to the
parent's stderr")

The GCC details are:

  ~$ gcc --version
  gcc (Ubuntu 4.4.1-4ubuntu9) 4.4.1

Re: build warnings

[Jeff King]

On Fri, Jan 29, 2010 at 08:03:37PM +1100, Michael Wookey wrote:

> With current master (dace5dd1), the following build warnings appear on
> Ubuntu 9.10 (x86):
> 
>   run-command.c: In function ‘notify_parent’:
>   run-command.c:70: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result
>   run-command.c: In function ‘die_child’:
>   run-command.c:80: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result
>   run-command.c:81: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result
>   run-command.c:82: warning: ignoring return value of ‘write’,
> declared with attribute warn_unused_result

There is no point in looking at the return value of any of those calls.
The first one is about notifying the parent process of a child's failure
to exec while it is dying (the surrounding function is even an atexit
handler!). If we can't do that, there is really no alternative behavior.
The latter three are printing fatal error messages. If we fail at that,
there is not much to do (unless we should print an error...).

>   ~$ gcc --version
>   gcc (Ubuntu 4.4.1-4ubuntu9) 4.4.1

I have heard that Ubuntu recently switched on unused result warnings by
default, and I have seen complaints that it is generating a lot of
uninteresting warnings like these.

Does anybody know if this behavior is here to stay? Can it be worked
around with -Wno-warn-unused-result or something? There are few enough
callsites here that I am not entirely opposed to annotating them with
"(void)write" (does that actually work?), but I worry that this is a
slippery slope. There are a lot of other calls whose return values are
also uninteresting (just looking in the vicinity of this code, I see an
fflush and a close, neither of whose failure would be interesting). I'm
not excited at the prospect of annotating all of them.

-Peff

Re: build warnings

[Erik Faye-Lund]

On Fri, Jan 29, 2010 at 12:02 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 29, 2010 at 08:03:37PM +1100, Michael Wookey wrote:
>
>> With current master (dace5dd1), the following build warnings appear on
>> Ubuntu 9.10 (x86):
>>
>>   run-command.c: In function ‘notify_parent’:
>>   run-command.c:70: warning: ignoring return value of ‘write’,
>> declared with attribute warn_unused_result
>>   run-command.c: In function ‘die_child’:
>>   run-command.c:80: warning: ignoring return value of ‘write’,
>> declared with attribute warn_unused_result
>>   run-command.c:81: warning: ignoring return value of ‘write’,
>> declared with attribute warn_unused_result
>>   run-command.c:82: warning: ignoring return value of ‘write’,
>> declared with attribute warn_unused_result
>
> There is no point in looking at the return value of any of those calls.
> The first one is about notifying the parent process of a child's failure
> to exec while it is dying (the surrounding function is even an atexit
> handler!). If we can't do that, there is really no alternative behavior.
> The latter three are printing fatal error messages. If we fail at that,
> there is not much to do (unless we should print an error...).
>
>>   ~$ gcc --version
>>   gcc (Ubuntu 4.4.1-4ubuntu9) 4.4.1
>
> I have heard that Ubuntu recently switched on unused result warnings by
> default, and I have seen complaints that it is generating a lot of
> uninteresting warnings like these.
>
> Does anybody know if this behavior is here to stay? Can it be worked
> around with -Wno-warn-unused-result or something? There are few enough
> callsites here that I am not entirely opposed to annotating them with
> "(void)write" (does that actually work?), but I worry that this is a
> slippery slope. There are a lot of other calls whose return values are
> also uninteresting (just looking in the vicinity of this code, I see an
> fflush and a close, neither of whose failure would be interesting). I'm
> not excited at the prospect of annotating all of them.
>

In my experience, quieting warn-unused-result globally isn't ideal;
this warning has helped me track down some serious issues many times
in the past. IIRC, gcc requires a specific attribute on a function
prototype in order to generate warnings when the return-value isn't
used. I guess the issue we're seeing here is that the glibc that
Ubuntu ships has recently added this attribute for some CRT-functions.

Personally I think that quieting them ("(void)func(...)" does work
AFAIK) can make sense as long as there's only a few call-sites.
However, if it's so many that it'll generate substantial noise in the
git-sources or Ubuntu-users will get annoyed beyond sense,
"-Wno-warn-unused-result" might be the only choice. But perhaps this
is something Ubuntu-users would put in their config.mak?

-- 
Erik "kusma" Faye-Lund