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