Re: [Qemu-devel] [PATCH] audio: Fix using freed pointer in wav_fini_out

[Gonglei (Arei)]

Hi,

> -----Original Message-----
> From: Paolo Bonzini [mailto:address@hidden
> Sent: Friday, June 13, 2014 4:18 PM
> To: Gerd Hoffmann
> Cc: Markus Armbruster; Gonglei (Arei); address@hidden;
> Huangweidong (C); Luonengjun; address@hidden
> Subject: Re: [Qemu-devel] [PATCH] audio: Fix using freed pointer in 
> wav_fini_out
> 
> Il 13/06/2014 09:59, Gerd Hoffmann ha scritto:
> > On Fr, 2014-06-13 at 09:47 +0200, Paolo Bonzini wrote:
> >> Il 13/06/2014 09:15, Markus Armbruster ha scritto:
> >>> I'm afraid this is not an improvement.
> >>>
> >>> Your patch makes the code ignore fclose() failure silently.  This is a
> >>> common mistake.  fclose() failure after write can mean data loss, and
> >>> the user certainly needs to know about that.
> >>
> >> If you want that, the best solution is to first fflush() and then
> >> fclose().
> >
> > Agree.  If you really care you'll go flush/sync before close, so you
> > still have a valid file handle when you see the failure.
> >
> > Question is do we really need that here?  This isn't your virtual disk,
> > it's just a few audio samples which might get lost ...
> 
> I think sticking to fflush+fclose is a good idea anyway, if only for
> discipline.  If Gonglei wants to do that, why not. :)
> 
> Otherwise, it's easy to mark the bug as intentional in Coverity.
> 
> Paolo

Thanks, you guys. 

Actually I search the whole qemu project before I post the patch,
and I find most module are just simply close the file handle, 
so I follow them.

For this issue, if you think we should stick to flush+fclose is a good idea,
I'd like to post another patch. I'm fine. Thanks.


Best regards,
-Gonglei