[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] audio: Fix using freed pointer in wav_fini_out
From: |
Gonglei (Arei) |
Subject: |
Re: [Qemu-devel] [PATCH] audio: Fix using freed pointer in wav_fini_out |
Date: |
Fri, 13 Jun 2014 08:32:11 +0000 |
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