[PATCH] winegstreamer: Fix memory leak in amt_from_gst_caps_video.

Andrew Eikum aeikum at codeweavers.com
Tue Nov 15 08:08:47 CST 2016


This is an improvement, but I think we should go further and fix leaks
in all of the failure cases. The problem is we're using the
amt_from_gst_caps_video return value inconsistently. In
accept_caps_sink, we unconditionally free the resulting AM_MEDIA_TYPE.
But in setcaps_sink, we only free it, via the pin->pmt assignment, if
amt_from_gst_caps_video succeeds.

I think we should always free the allocated memory in the failure
cases in amt_from_gst_caps_video, and then only free the resulting AMT
in the calling functions if amt_from_gst_caps_video succeeded. The
resulting AMT should never be used or freed if amt_from_gst_caps_video
fails.

Note that amt_from_gst_caps_audio has the same inconsistent return
type handling. I would accept a separate patch to fix this handling,
too. (Unlike for video, it doesn't currently matter in practice
because of amt_from_gst_caps_audio's implementation.)

Does that make sense?

Andrew

On Mon, Nov 14, 2016 at 11:43:21PM -0700, Alex Henrie wrote:
> Cc: Andrew Eikum <aeikum at codeweavers.com>
> 
> Coverity #713781, "Variable vih going out of scope leaks the storage it
> points to."
> 
> Signed-off-by: Alex Henrie <alexhenrie24 at gmail.com>
> ---
>  dlls/winegstreamer/gstdemux.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c
> index a6a41ab..5d97510 100644
> --- a/dlls/winegstreamer/gstdemux.c
> +++ b/dlls/winegstreamer/gstdemux.c
> @@ -179,18 +179,21 @@ static gboolean amt_from_gst_caps_audio(GstCaps *caps, AM_MEDIA_TYPE *amt)
>  
>  static gboolean amt_from_gst_caps_video(GstCaps *caps, AM_MEDIA_TYPE *amt)
>  {
> -    VIDEOINFOHEADER *vih = CoTaskMemAlloc(sizeof(*vih));
> -    BITMAPINFOHEADER *bih = &vih->bmiHeader;
> +    VIDEOINFOHEADER *vih;
> +    BITMAPINFOHEADER *bih;
>      gint32 width = 0, height = 0, nom = 0, denom = 0;
>      GstVideoInfo vinfo;
>  
>      if (!gst_video_info_from_caps (&vinfo, caps))
>          return FALSE;
>      width = vinfo.width;
>      height = vinfo.height;
>      nom = vinfo.fps_n;
>      denom = vinfo.fps_d;
>  
> +    vih = CoTaskMemAlloc(sizeof(*vih));
> +    bih = &vih->bmiHeader;
> +
>      amt->formattype = FORMAT_VideoInfo;
>      amt->pbFormat = (BYTE*)vih;
>      amt->cbFormat = sizeof(*vih);
> -- 
> 2.10.2
> 



More information about the wine-devel mailing list