[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result
From: |
Felipe Franciosi |
Subject: |
Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result |
Date: |
Wed, 21 Sep 2016 10:00:23 +0000 |
> On 21 Sep 2016, at 07:24, Markus Armbruster <address@hidden> wrote:
>
> "Pavel Dovgalyuk" <address@hidden> writes:
>
>>> From: Felipe Franciosi [mailto:address@hidden
>>> 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 <address@hidden>
>>> ---
>>> 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
- [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Felipe Franciosi, 2016/09/21
- Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Pavel Dovgalyuk, 2016/09/21
- Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Markus Armbruster, 2016/09/21
- Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Markus Armbruster, 2016/09/21
- Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Eric Blake, 2016/09/21
- Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Felipe Franciosi, 2016/09/21
- Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Daniel P. Berrange, 2016/09/21
- Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Felipe Franciosi, 2016/09/21
- Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result, Eric Blake, 2016/09/21