Re: [Qemu-devel] [PATCH v3 3/4] audio: Fix using freed pointer in wav_fini_out()

[Peter Maydell]

On 10 June 2014 10:20,  <address@hidden> wrote:
> 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));

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 <address@hidden>
> Reviewed-by: Paolo Bonzini <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> ---
>  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