Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result

[Felipe Franciosi]

> On 21 Sep 2016, at 15:35, Daniel P. Berrange <address@hidden> wrote:
> 
> On Wed, Sep 21, 2016 at 02:26:48PM +0000, Felipe Franciosi wrote:
>> 
>>> On 21 Sep 2016, at 14:55, Eric Blake <address@hidden> wrote:
>>> 
>>> On 09/21/2016 07:31 AM, Markus Armbruster wrote:
>>>>> 
>>>>> If we want to ignore return value reliably, lets just pull in the
>>>>> ignore_value macro from gnulib which is known to work across GCC
>>>>> versions
>>>>> 
>>>>> 
>>>>> /* Normally casting an expression to void discards its value, but GCC
>>>>>  versions 3.4 and newer have __attribute__ ((__warn_unused_result__))
>>>>>  which may cause unwanted diagnostics in that case.  Use __typeof__
>>>>>  and __extension__ to work around the problem, if the workaround is
>>>>>  known to be needed.  */
>>>>> #if 3 < __GNUC__ + (4 <= __GNUC_MINOR__)
>>>>> # define ignore_value(x) \
>>>>>   (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
>>>>> #else
>>>>> # define ignore_value(x) ((void) (x))
>>>>> #endif
>>>> 
>>>> Casting a value to void is the traditional and obvious way to say "yes,
>>>> I mean to ignore this value".  Now compilers start to reply "no, you
>>>> don't".  We can invent new (and less obvious) ways to say "yes, I do",
>>>> and compilers can then learn them so they can again reply "no, you
>>>> don't".  Why have compilers started to behave like two-year-olds?
>>> 
>>> gcc has been doing the "__warn_unused_value__ means cast-to-void is
>>> insufficient" complaint for years (since at least 2008, per the gnulib
>>> history).  But the gnulib workaround has also been effectively silencing
>>> it for years (it was actually my work in 2011, commit 939dedd, which
>>> came up with the form listed above).  The other nice thing about
>>> "ignore_value(wur_function())" is that you are avoiding a cast in your
>>> local code, and the burden of shutting up the annoying compiler is
>>> hidden behind a macro that can easily be changed to affect all clients
>>> of the macro, should gcc regress yet again and we need some other
>>> formula to shut it up.
>>> 
>>> And yes, the gnulib mailing list has threads complaining about gcc's
>>> behavior back when the macro had to be invented, and again when glibc
>>> added wur markings to functions that can legitimately be ignored
>>> (fread() is one of them; because there are valid programming paradigms
>>> where you check ferror() later on rather than having to check every
>>> intermediate fread(), at the expense of less-specific error messages).
>> 
>> What's the best way to bring gnulib's ignore-value.h into Qemu? I'd think we 
>> could just add to include/qemu/compiler.h something like:
>> 
>> ----------------------8<----------------------
>> #if QEMU_GNUC_PREREQ(3, 4)
>> /* From gnulib's ignore-value.h by Jim Meyering, Eric Blake and Padraig 
>> Brady */
>> # define ignore_value(x) \,
>>         (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
>> #else
>> # define ignore_value(x) ((void) (x))
>> #endif
>> ----------------------8<----------------------
>> 
>> But I'm not sure if that suffices to meet GPL's requirements.
> 
> The compiler.h file has no license header, just a comment
> saying "public domain", which is obviously not the case
> if you add this macro.
> 
> Given that you'll need to explicitly mention the license terms
> for ignore_value. eg with a comment line like
> 
>  /* The ignore_value() macro is taken from GNULIB ignore-value.h,
>   * licensed under the terms of the LGPLv2+
>   */

Awesome, thanks!
I think it's better to fit it in compiler.h than to add a separate header file, 
which would then need to be included by any potential user.

Felipe

> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|