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(a)codeweavers.com>
>
> Coverity #713781, "Variable vih going out of scope leaks the storage it
> points to."
>
> Signed-off-by: Alex Henrie <alexhenrie24(a)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
>