build warnings

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

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

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...).

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

On Fri, Jan 29, 2010 at 12:02 PM, Jeff King <peff@peff.net> wrote:

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?