[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: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] audio: Fix using freed pointer in wav_fini_out |
Date: |
Fri, 13 Jun 2014 09:15:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
<address@hidden> writes:
> From: Gonglei <address@hidden>
>
> 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));
False positive, because dolog() doesn't dereference wav->f, it only
prints it.
> Drop the whole message, we can't do much when it happen after all.
>
> Signed-off-by: Gonglei <address@hidden>
> Reviewed-by: Paolo Bonzini <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> ---
> audio/wavaudio.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/audio/wavaudio.c b/audio/wavaudio.c
> index 6846a1a..b81fdf9 100644
> --- a/audio/wavaudio.c
> +++ b/audio/wavaudio.c
> @@ -209,10 +209,7 @@ 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));
> - }
> + fclose(wav->f);
> wav->f = NULL;
>
> g_free (wav->pcm_buf);
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.