| Message ID | 1402392027-9164-4-git-send-email-arei.gonglei@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 10 June 2014 10:20, <arei.gonglei@huawei.com> wrote: > From: Gonglei <arei.gonglei@huawei.com> > > Spotted by Coverity: > > (8) Event freed_arg: "fclose(FILE *)" frees "wav->f". > (9) Event cond_true: Condition "fclose(wav->f)", taking true branch > Also see events: [pass_freed_arg] > > 212 if (fclose (wav->f)) { > (10) Event pass_freed_arg: Passing freed pointer "wav->f" as an argument > to function "AUD_log(char const *, char const *, ...)". > Also see events: [freed_arg] > > 213 dolog ("wav_fini_out: fclose %p failed\nReason: %s\n", > 214 wav->f, strerror (errno)); This is actually a coverity false positive -- we're not using the freed pointer, just printing its value. However, there's not much value in printing the pointer value to the error log, especially since we don't print the pointer value on fopen() or any other logging, so you can't match up the fclose pointer with anything else. > Removed wav->f's pointer in error log, actually it's uselessly. > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > audio/wavaudio.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/audio/wavaudio.c b/audio/wavaudio.c > index 6846a1a..9bbe8e9 100644 > --- a/audio/wavaudio.c > +++ b/audio/wavaudio.c > @@ -210,8 +210,8 @@ static void wav_fini_out (HWVoiceOut *hw) > > doclose: > if (fclose (wav->f)) { > - dolog ("wav_fini_out: fclose %p failed\nReason: %s\n", > - wav->f, strerror (errno)); > + dolog ("wav_fini_out: fclose 'wav->f' failed\nReason: %s\n", > + strerror (errno)); I would just drop the 'wav->f' here. > } > wav->f = NULL; > > -- thanks -- PMM
Hi, > > doclose: > > if (fclose (wav->f)) { > > - dolog ("wav_fini_out: fclose %p failed\nReason: %s\n", > > - wav->f, strerror (errno)); > > + dolog ("wav_fini_out: fclose 'wav->f' failed\nReason: %s\n", > > + strerror (errno)); > > I would just drop the 'wav->f' here. Or drop the whole message? It's a highly unlikely error condition after all, and we can't do much about it anyway. cheers, Gerd
> -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: Thursday, June 12, 2014 6:32 PM > To: Peter Maydell > Cc: Gonglei (Arei); QEMU Developers; Huangweidong (C); Luonengjun; Luiz > Capitulino; Vassili Karpov; Stefan Hajnoczi; Paolo Bonzini > Subject: Re: [Qemu-devel] [PATCH v3 3/4] audio: Fix using freed pointer in > wav_fini_out() > > Hi, > > > > doclose: > > > if (fclose (wav->f)) { > > > - dolog ("wav_fini_out: fclose %p failed\nReason: %s\n", > > > - wav->f, strerror (errno)); > > > + dolog ("wav_fini_out: fclose 'wav->f' failed\nReason: %s\n", > > > + strerror (errno)); > > > > I would just drop the 'wav->f' here. > > Or drop the whole message? It's a highly unlikely error condition after > all, and we can't do much about it anyway. > Agreed. Gerd, would you like a new version? Best regards, -Gonglei
> - dolog ("wav_fini_out: fclose %p failed\nReason: %s\n", > > > > - wav->f, strerror (errno)); > > > > + dolog ("wav_fini_out: fclose 'wav->f' failed\nReason: %s\n", > > > > + strerror (errno)); > > > > > > I would just drop the 'wav->f' here. > > > > Or drop the whole message? It's a highly unlikely error condition after > > all, and we can't do much about it anyway. > > > Agreed. Gerd, would you like a new version? yes, please. thanks, Gerd
diff --git a/audio/wavaudio.c b/audio/wavaudio.c index 6846a1a..9bbe8e9 100644 --- a/audio/wavaudio.c +++ b/audio/wavaudio.c @@ -210,8 +210,8 @@ static void wav_fini_out (HWVoiceOut *hw) doclose: if (fclose (wav->f)) { - dolog ("wav_fini_out: fclose %p failed\nReason: %s\n", - wav->f, strerror (errno)); + dolog ("wav_fini_out: fclose 'wav->f' failed\nReason: %s\n", + strerror (errno)); } wav->f = NULL;