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

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

[Felipe Franciosi]

If compiling with -Werror=unused-result, replay-internal.c won't build
due to a call to fwrite() where the returned value is ignored. A simple
cast to (void) is not sufficient on recent GCCs, so this fixes it.

Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
---
 replay/replay-internal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index 5835e8d..6978d76 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
 {
     if (replay_file) {
         replay_put_dword(size);
-        fwrite(buf, 1, size, replay_file);
+        (void)(fwrite(buf, 1, size, replay_file)+1);
     }
 }
 
-- 
1.9.5

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

[Pavel Dovgalyuk]

> From: Felipe Franciosi [mailto:felipe@nutanix.com]
> If compiling with -Werror=unused-result, replay-internal.c won't build
> due to a call to fwrite() where the returned value is ignored. A simple
> cast to (void) is not sufficient on recent GCCs, so this fixes it.
> 
> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
> ---
>  replay/replay-internal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> index 5835e8d..6978d76 100644
> --- a/replay/replay-internal.c
> +++ b/replay/replay-internal.c
> @@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
>  {
>      if (replay_file) {
>          replay_put_dword(size);
> -        fwrite(buf, 1, size, replay_file);
> +        (void)(fwrite(buf, 1, size, replay_file)+1);
>      }
>  }

This looks very weird.
I think it would be better to check the return value and stop
the simulation in case of error.

Pavel Dovgalyuk

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

[Markus Armbruster]

"Pavel Dovgalyuk" <dovgaluk@ispras.ru> writes:

>> From: Felipe Franciosi [mailto:felipe@nutanix.com]
>> If compiling with -Werror=unused-result, replay-internal.c won't build
>> due to a call to fwrite() where the returned value is ignored. A simple
>> cast to (void) is not sufficient on recent GCCs, so this fixes it.
>> 
>> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
>> ---
>>  replay/replay-internal.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
>> index 5835e8d..6978d76 100644
>> --- a/replay/replay-internal.c
>> +++ b/replay/replay-internal.c
>> @@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
>>  {
>>      if (replay_file) {
>>          replay_put_dword(size);
>> -        fwrite(buf, 1, size, replay_file);
>> +        (void)(fwrite(buf, 1, size, replay_file)+1);
>>      }
>>  }
>
> This looks very weird.
> I think it would be better to check the return value and stop
> the simulation in case of error.

Ignoring errors is wrong more often than not.  Whether it's wrong in
this case I can't say.  What I can say is 1. the commit message needs to
explain *why* the error can be ignored, and 2. the way you ignore the
error is scandalously ugly :)  You say gcc doesn't honor the traditional
cast to void anymore.  What's the exact error message you see?

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

[Felipe Franciosi]

> On 21 Sep 2016, at 07:24, Markus Armbruster <armbru@redhat.com> wrote:
> 
> "Pavel Dovgalyuk" <dovgaluk@ispras.ru> writes:
> 
>>> From: Felipe Franciosi [mailto:felipe@nutanix.com]
>>> If compiling with -Werror=unused-result, replay-internal.c won't build
>>> due to a call to fwrite() where the returned value is ignored. A simple
>>> cast to (void) is not sufficient on recent GCCs, so this fixes it.
>>> 
>>> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
>>> ---
>>> replay/replay-internal.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
>>> index 5835e8d..6978d76 100644
>>> --- a/replay/replay-internal.c
>>> +++ b/replay/replay-internal.c
>>> @@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
>>> {
>>>     if (replay_file) {
>>>         replay_put_dword(size);
>>> -        fwrite(buf, 1, size, replay_file);
>>> +        (void)(fwrite(buf, 1, size, replay_file)+1);
>>>     }
>>> }
>> 
>> This looks very weird.

Oh I couldn't agree more. I hate this syntax. It is, however, the simplest way to get around this issue. See:
http://stackoverflow.com/questions/11888594/ignoring-return-values-in-c

That doesn't mean we should do it. I actually rather we didn't. :)
I sent this patch to discuss how the maintainers feel about it given no one bumped into this yet (apparently).

>> I think it would be better to check the return value and stop
>> the simulation in case of error.

We can also make replay_put_array() return an int indicating whether an error occurred. The callers can then decide whether to care or not. I'll send a v2 doing that instead for comments.

> Ignoring errors is wrong more often than not.  Whether it's wrong in
> this case I can't say.

True that. That's why I think a better first step is to make functions that can fail return an error code. Callers can make that decision later. Unfortunately, glibc decided that fwrite() should be tagged with WUR and that triggers this build failure.

> What I can say is 1. the commit message needs to
> explain *why* the error can be ignored,

The error is already ignored today. I'll send a v2 that makes replay_put_array() reflect the error, the callers can then decide to ignore it.

> and 2. the way you ignore the
> error is scandalously ugly :)

Oh we all agree on that. :-D

> You say gcc doesn't honor the traditional
> cast to void anymore.  What's the exact error message you see?

See this (lengthly discussion) for more details:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509

Thanks,
Felipe

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

[Pavel Dovgalyuk]

> From: Felipe Franciosi [mailto:felipe@nutanix.com]
> > On 21 Sep 2016, at 07:24, Markus Armbruster <armbru@redhat.com> wrote:
> >
> > "Pavel Dovgalyuk" <dovgaluk@ispras.ru> writes:
> >
> >>> From: Felipe Franciosi [mailto:felipe@nutanix.com]
> >>> If compiling with -Werror=unused-result, replay-internal.c won't build
> >>> due to a call to fwrite() where the returned value is ignored. A simple
> >>> cast to (void) is not sufficient on recent GCCs, so this fixes it.
> >>>
> >>> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
> >>> ---
> >>> replay/replay-internal.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> >>> index 5835e8d..6978d76 100644
> >>> --- a/replay/replay-internal.c
> >>> +++ b/replay/replay-internal.c
> >>> @@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
> >>> {
> >>>     if (replay_file) {
> >>>         replay_put_dword(size);
> >>> -        fwrite(buf, 1, size, replay_file);
> >>> +        (void)(fwrite(buf, 1, size, replay_file)+1);
> >>>     }
> >>> }
> >>
> >> This looks very weird.
> 
> >> I think it would be better to check the return value and stop
> >> the simulation in case of error.
> 
> We can also make replay_put_array() return an int indicating whether an error occurred. The
> callers can then decide whether to care or not. I'll send a v2 doing that instead for
> comments.
> 
> > Ignoring errors is wrong more often than not.  Whether it's wrong in
> > this case I can't say.
> 
> True that. That's why I think a better first step is to make functions that can fail return an
> error code. Callers can make that decision later. Unfortunately, glibc decided that fwrite()
> should be tagged with WUR and that triggers this build failure.
> 
> > What I can say is 1. the commit message needs to
> > explain *why* the error can be ignored,
> 
> The error is already ignored today. I'll send a v2 that makes replay_put_array() reflect the
> error, the callers can then decide to ignore it.

There is no need to return an error.
When rr log cannot be created there is no need in executing in record mode.
Therefore execution has to be stopped in case of an error.

Pavel Dovgalyuk

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

[Daniel P. Berrange]

On Wed, Sep 21, 2016 at 10:00:23AM +0000, Felipe Franciosi wrote:
> 
> > On 21 Sep 2016, at 07:24, Markus Armbruster <armbru@redhat.com> wrote:
> > 
> > "Pavel Dovgalyuk" <dovgaluk@ispras.ru> writes:
> > 
> >>> From: Felipe Franciosi [mailto:felipe@nutanix.com]
> >>> If compiling with -Werror=unused-result, replay-internal.c won't build
> >>> due to a call to fwrite() where the returned value is ignored. A simple
> >>> cast to (void) is not sufficient on recent GCCs, so this fixes it.
> >>> 
> >>> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
> >>> ---
> >>> replay/replay-internal.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> >>> index 5835e8d..6978d76 100644
> >>> --- a/replay/replay-internal.c
> >>> +++ b/replay/replay-internal.c
> >>> @@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
> >>> {
> >>>     if (replay_file) {
> >>>         replay_put_dword(size);
> >>> -        fwrite(buf, 1, size, replay_file);
> >>> +        (void)(fwrite(buf, 1, size, replay_file)+1);
> >>>     }
> >>> }
> >> 
> >> This looks very weird.
> 
> Oh I couldn't agree more. I hate this syntax. It is, however, the simplest way to get around this issue. See:
> http://stackoverflow.com/questions/11888594/ignoring-return-values-in-c

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

GNULIB makes it availavble under LGPLv2.1+

eg used as:

   ignore_value(fwrite(buf, 1, size, replay_file));

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 :|

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

[Felipe Franciosi]

> On 21 Sep 2016, at 11:07, Daniel P. Berrange <berrange@redhat.com> wrote:
> 
> On Wed, Sep 21, 2016 at 10:00:23AM +0000, Felipe Franciosi wrote:
>> 
>>> On 21 Sep 2016, at 07:24, Markus Armbruster <armbru@redhat.com> wrote:
>>> 
>>> "Pavel Dovgalyuk" <dovgaluk@ispras.ru> writes:
>>> 
>>>>> From: Felipe Franciosi [mailto:felipe@nutanix.com]
>>>>> If compiling with -Werror=unused-result, replay-internal.c won't build
>>>>> due to a call to fwrite() where the returned value is ignored. A simple
>>>>> cast to (void) is not sufficient on recent GCCs, so this fixes it.
>>>>> 
>>>>> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
>>>>> ---
>>>>> replay/replay-internal.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
>>>>> index 5835e8d..6978d76 100644
>>>>> --- a/replay/replay-internal.c
>>>>> +++ b/replay/replay-internal.c
>>>>> @@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
>>>>> {
>>>>>    if (replay_file) {
>>>>>        replay_put_dword(size);
>>>>> -        fwrite(buf, 1, size, replay_file);
>>>>> +        (void)(fwrite(buf, 1, size, replay_file)+1);
>>>>>    }
>>>>> }
>>>> 
>>>> This looks very weird.
>> 
>> Oh I couldn't agree more. I hate this syntax. It is, however, the simplest way to get around this issue. See:
>> http://stackoverflow.com/questions/11888594/ignoring-return-values-in-c
> 
> 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

I like that better. Do you want a series with a patch to add this macro to include/qemu/compiler.h and another making replay_put_array() use it?

Cheers,
Felipe

> 
> 
> /* 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
> 
> GNULIB makes it availavble under LGPLv2.1+
> 
> eg used as:
> 
>   ignore_value(fwrite(buf, 1, size, replay_file));
> 
> 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 :|

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

[Felipe Franciosi]

Actually, I just noticed Pavel is in the middle of submitting a "replay additions" series (currently at v4).

Pavel: is this something you can address as part of that series?

Thanks,
Felipe

> On 21 Sep 2016, at 11:12, Felipe Franciosi <felipe@nutanix.com> wrote:
> 
> 
>> On 21 Sep 2016, at 11:07, Daniel P. Berrange <berrange@redhat.com> wrote:
>> 
>> On Wed, Sep 21, 2016 at 10:00:23AM +0000, Felipe Franciosi wrote:
>>> 
>>>> On 21 Sep 2016, at 07:24, Markus Armbruster <armbru@redhat.com> wrote:
>>>> 
>>>> "Pavel Dovgalyuk" <dovgaluk@ispras.ru> writes:
>>>> 
>>>>>> From: Felipe Franciosi [mailto:felipe@nutanix.com]
>>>>>> If compiling with -Werror=unused-result, replay-internal.c won't build
>>>>>> due to a call to fwrite() where the returned value is ignored. A simple
>>>>>> cast to (void) is not sufficient on recent GCCs, so this fixes it.
>>>>>> 
>>>>>> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
>>>>>> ---
>>>>>> replay/replay-internal.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>> 
>>>>>> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
>>>>>> index 5835e8d..6978d76 100644
>>>>>> --- a/replay/replay-internal.c
>>>>>> +++ b/replay/replay-internal.c
>>>>>> @@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
>>>>>> {
>>>>>>   if (replay_file) {
>>>>>>       replay_put_dword(size);
>>>>>> -        fwrite(buf, 1, size, replay_file);
>>>>>> +        (void)(fwrite(buf, 1, size, replay_file)+1);
>>>>>>   }
>>>>>> }
>>>>> 
>>>>> This looks very weird.
>>> 
>>> Oh I couldn't agree more. I hate this syntax. It is, however, the simplest way to get around this issue. See:
>>> http://stackoverflow.com/questions/11888594/ignoring-return-values-in-c
>> 
>> 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
> 
> I like that better. Do you want a series with a patch to add this macro to include/qemu/compiler.h and another making replay_put_array() use it?
> 
> Cheers,
> Felipe
> 
>> 
>> 
>> /* 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
>> 
>> GNULIB makes it availavble under LGPLv2.1+
>> 
>> eg used as:
>> 
>>  ignore_value(fwrite(buf, 1, size, replay_file));
>> 
>> 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 :|
> 

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

[Markus Armbruster]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Wed, Sep 21, 2016 at 10:00:23AM +0000, Felipe Franciosi wrote:
>> 
>> > On 21 Sep 2016, at 07:24, Markus Armbruster <armbru@redhat.com> wrote:
>> > 
>> > "Pavel Dovgalyuk" <dovgaluk@ispras.ru> writes:
>> > 
>> >>> From: Felipe Franciosi [mailto:felipe@nutanix.com]
>> >>> If compiling with -Werror=unused-result, replay-internal.c won't build
>> >>> due to a call to fwrite() where the returned value is ignored. A simple
>> >>> cast to (void) is not sufficient on recent GCCs, so this fixes it.
>> >>> 
>> >>> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
>> >>> ---
>> >>> replay/replay-internal.c | 2 +-
>> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>> 
>> >>> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
>> >>> index 5835e8d..6978d76 100644
>> >>> --- a/replay/replay-internal.c
>> >>> +++ b/replay/replay-internal.c
>> >>> @@ -65,7 +65,7 @@ void replay_put_array(const uint8_t *buf, size_t size)
>> >>> {
>> >>>     if (replay_file) {
>> >>>         replay_put_dword(size);
>> >>> -        fwrite(buf, 1, size, replay_file);
>> >>> +        (void)(fwrite(buf, 1, size, replay_file)+1);
>> >>>     }
>> >>> }
>> >> 
>> >> This looks very weird.
>> 
>> Oh I couldn't agree more. I hate this syntax. It is, however, the simplest way to get around this issue. See:
>> http://stackoverflow.com/questions/11888594/ignoring-return-values-in-c
>
> 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?

[...]

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2303 bytes --]

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Felipe Franciosi]

> On 21 Sep 2016, at 14:55, Eric Blake <eblake@redhat.com> 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.

Thanks,
Felipe

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

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

[Daniel P. Berrange]

On Wed, Sep 21, 2016 at 02:26:48PM +0000, Felipe Franciosi wrote:
> 
> > On 21 Sep 2016, at 14:55, Eric Blake <eblake@redhat.com> 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+
   */

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 :|

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

[Felipe Franciosi]

> On 21 Sep 2016, at 15:35, Daniel P. Berrange <berrange@redhat.com> wrote:
> 
> On Wed, Sep 21, 2016 at 02:26:48PM +0000, Felipe Franciosi wrote:
>> 
>>> On 21 Sep 2016, at 14:55, Eric Blake <eblake@redhat.com> 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 :|

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

On 09/21/2016 09:35 AM, Daniel P. Berrange wrote:

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

Oh, I missed that; I was assuming compiler.h was GPLv2+ per the usual
project defaults; but that file is explicitly looser.

> 
> 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+
>    */

Yes, adding that solves any questions of origin.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Markus Armbruster]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Wed, Sep 21, 2016 at 02:26:48PM +0000, Felipe Franciosi wrote:
>> 
>> > On 21 Sep 2016, at 14:55, Eric Blake <eblake@redhat.com> 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+
>    */

Our tree has a mix of licenses, which is enough of a pain.  Mixing
licenses within *files* is even worse, and might not even be legally
sound.

Relicense the whole file under our preferred license GPLv2+?

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1056 bytes --]

On 09/21/2016 10:28 AM, Markus Armbruster wrote:

>> 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+
>>    */
> 
> Our tree has a mix of licenses, which is enough of a pain.  Mixing
> licenses within *files* is even worse, and might not even be legally
> sound.
> 
> Relicense the whole file under our preferred license GPLv2+?

That works too.  No one can legally complain - the current license is so
permissive that marking the entire file LGPLv2+ is permitted by the
current license.  It's a one-way conversion (we can't go back once we do
it), but I would be fine with that approach.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Daniel P. Berrange]

On Wed, Sep 21, 2016 at 01:18:58PM -0500, Eric Blake wrote:
> On 09/21/2016 10:28 AM, Markus Armbruster wrote:
> 
> >> 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+
> >>    */
> > 
> > Our tree has a mix of licenses, which is enough of a pain.  Mixing
> > licenses within *files* is even worse, and might not even be legally
> > sound.
> > 
> > Relicense the whole file under our preferred license GPLv2+?
> 
> That works too.  No one can legally complain - the current license is so
> permissive that marking the entire file LGPLv2+ is permitted by the
> current license.  It's a one-way conversion (we can't go back once we do
> it), but I would be fine with that approach.

I think the file probably should not have been listed as public domain
in the first place, as its initial contents were copied from qemu-common.h
which is not public domain.


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 :|

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

[Markus Armbruster]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Wed, Sep 21, 2016 at 01:18:58PM -0500, Eric Blake wrote:
>> On 09/21/2016 10:28 AM, Markus Armbruster wrote:
>> 
>> >> 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+
>> >>    */
>> > 
>> > Our tree has a mix of licenses, which is enough of a pain.  Mixing
>> > licenses within *files* is even worse, and might not even be legally
>> > sound.
>> > 
>> > Relicense the whole file under our preferred license GPLv2+?
>> 
>> That works too.  No one can legally complain - the current license is so
>> permissive that marking the entire file LGPLv2+ is permitted by the
>> current license.  It's a one-way conversion (we can't go back once we do
>> it), but I would be fine with that approach.
>
> I think the file probably should not have been listed as public domain
> in the first place, as its initial contents were copied from qemu-common.h
> which is not public domain.

Ewww!  Needs fixing.

Since qemu-common.h carries no license, GPLv2+ applies.

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 664 bytes --]

On 09/22/2016 06:51 AM, Markus Armbruster wrote:
>>
>> I think the file probably should not have been listed as public domain
>> in the first place, as its initial contents were copied from qemu-common.h
>> which is not public domain.
> 
> Ewww!  Needs fixing.

Indeed.  Commit 5c02632 shows the initial file creation, but Luiz used a
different email address at that time.  What would be the best line to
list as a copyright holder on both qemu-common.h and on compiler.h?

> 
> Since qemu-common.h carries no license, GPLv2+ applies.
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 09/22/2016 06:51 AM, Markus Armbruster wrote:
>>>
>>> I think the file probably should not have been listed as public domain
>>> in the first place, as its initial contents were copied from qemu-common.h
>>> which is not public domain.
>> 
>> Ewww!  Needs fixing.
>
> Indeed.  Commit 5c02632 shows the initial file creation, but Luiz used a
> different email address at that time.  What would be the best line to
> list as a copyright holder on both qemu-common.h and on compiler.h?

Since qemu-common.h carried no licence notice back then[*], adding the
line "/* public domain */" was to compiler.h was wrong, and is probably
legally void.  The immediate fix is therefore dropping that line.  Then
the project-wide license applies: GPLv2+.

We can always copy the standard GPLv2+ licence notice to files that
don't have one, if we think that's useful.

>> Since qemu-common.h carries no license, GPLv2+ applies.


[*] It still doesn't, but that's immaterial.

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

[Felipe Franciosi]

> On 23 Sep 2016, at 09:15, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 09/22/2016 06:51 AM, Markus Armbruster wrote:
>>>> 
>>>> I think the file probably should not have been listed as public domain
>>>> in the first place, as its initial contents were copied from qemu-common.h
>>>> which is not public domain.
>>> 
>>> Ewww!  Needs fixing.
>> 
>> Indeed.  Commit 5c02632 shows the initial file creation, but Luiz used a
>> different email address at that time.  What would be the best line to
>> list as a copyright holder on both qemu-common.h and on compiler.h?
> 
> Since qemu-common.h carried no licence notice back then[*], adding the
> line "/* public domain */" was to compiler.h was wrong, and is probably
> legally void.  The immediate fix is therefore dropping that line.  Then
> the project-wide license applies: GPLv2+.
> 
> We can always copy the standard GPLv2+ licence notice to files that
> don't have one, if we think that's useful.
> 
>>> Since qemu-common.h carries no license, GPLv2+ applies.
> 
> 
> [*] It still doesn't, but that's immaterial.

Seems like all of us agree on that, so I sent another patch just for this. Once it's in, we can go back to adding the 'ignore_value()' macro and figuring out what to do with 'replay'.

http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg06206.html

Cheers,
Felipe

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]

On 09/21/2016 09:26 AM, Felipe Franciosi wrote:
> 
> 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.

As Dan already pointed out, gnulib's ignore_value() is available under
LGPLv2+ (the full gnulib.git/lib/ignore-value.h file states GPLv3+, for
ease of copying into other GPLv3 projects; but the modules/ignore-value
file states what additional licenses it can be used under).  Using
'gnulib-tool --lgpl=2 ignore-value' would give you the modified version;
but it is probably overkill for this situation (we can skip straight to
the end result instead of going through intermediate steps).

Furthermore, I'm the original author of that code snippet as listed (as
gnulib.git will show); where the only modification made after I wrote it
was whitespace changes and the addition of __extension__, and I'm just
fine with those lines being used as-is in qemu.git (there's benefits
when an original author states intentions :)

So copying and pasting the relevant snippet into compiler.h is indeed
the way to go; in fact, you can probably get away with a comment that
just says "/* From gnulib's LGPLv2+ ignore-value.h */" without having to
call out particular authors.  Don't bother creating any new header or
worrying about copyright boilerplate changes; and you can point to this
message-id in the commit message if you are worried about a paper trail;
you are not violating the GPL in this instance.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1474391326-871-1-git-send-email-felipe@nutanix.com
Subject: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/1473591360-13163-1-git-send-email-caoj.fnst@cn.fujitsu.com -> patchew/1473591360-13163-1-git-send-email-caoj.fnst@cn.fujitsu.com
 * [new tag]         patchew/1474391326-871-1-git-send-email-felipe@nutanix.com -> patchew/1474391326-871-1-git-send-email-felipe@nutanix.com
 - [tag update]      patchew/1474432046-325-1-git-send-email-famz@redhat.com -> patchew/1474432046-325-1-git-send-email-famz@redhat.com
Switched to a new branch 'test'
4439fa6 replay: Fix build with -Werror=unused-result

=== OUTPUT BEGIN ===
Checking PATCH 1/1: replay: Fix build with -Werror=unused-result...
ERROR: spaces required around that '+' (ctx:VxV)
#22: FILE: replay/replay-internal.c:68:
+        (void)(fwrite(buf, 1, size, replay_file)+1);
                                                 ^

total: 1 errors, 0 warnings, 8 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org