From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47544) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmeNY-0001nP-8Z for qemu-devel@nongnu.org; Wed, 21 Sep 2016 06:03:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bmeNS-0001LQ-02 for qemu-devel@nongnu.org; Wed, 21 Sep 2016 06:03:47 -0400 Received: from mail.ispras.ru ([83.149.199.45]:51852) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmeNR-0001JE-Oz for qemu-devel@nongnu.org; Wed, 21 Sep 2016 06:03:41 -0400 From: "Pavel Dovgalyuk" References: <1474391326-871-1-git-send-email-felipe@nutanix.com> <000301d213c7$7142f980$53c8ec80$@ru> <87y42l93dr.fsf@dusky.pond.sub.org> In-Reply-To: Date: Wed, 21 Sep 2016 13:03:26 +0300 Message-ID: <000301d213ef$65eb8110$31c28330$@ru> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Language: ru Subject: Re: [Qemu-devel] [PATCH] replay: Fix build with -Werror=unused-result List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Felipe Franciosi' , 'Markus Armbruster' Cc: 'Pavel Dovgalyuk' , 'Eric Blake' , 'qemu-devel' > From: Felipe Franciosi [mailto:felipe@nutanix.com] > > On 21 Sep 2016, at 07:24, Markus Armbruster wrote: > > > > "Pavel Dovgalyuk" 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 > >>> --- > >>> 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