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

[Pavel Dovgalyuk]

> From: Felipe Franciosi [mailto:address@hidden
> > 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.
> 
> >> 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