After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769299 - ogg: check return values in gst_ogg_parse_new_stream
ogg: check return values in gst_ogg_parse_new_stream
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal minor
: 1.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-07-29 11:18 UTC by Luis de Bethencourt
Modified: 2016-08-03 17:06 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
patch (1.51 KB, patch)
2016-07-29 11:18 UTC, Luis de Bethencourt 		none Details | Review
patch that fixes original memory leak (841 bytes, patch)
2016-08-01 15:05 UTC, Luis de Bethencourt 		committed Details | Review
version 2 of patch to check return values (2.04 KB, patch)
2016-08-01 15:05 UTC, Luis de Bethencourt 		committed Details | Review

Description Luis de Bethencourt 2016-07-29 11:18:15 UTC 
Created attachment 332346 [details] [review]
patch

There were two FIXME comments about checking the return values in gst_ogg_parse_new_stream.

This function already returned NULL if ogg_stream_init() failed, but gst_ogg_parse_chain() didn't check and just assumed _new_stream succeeded.

Submitting the patch here for comments. Maybe the ERROR messages can be improved :)
Comment 1 Vincent Penquerc'h 2016-08-01 12:19:27 UTC 
Review of attachment 332346 [details] [review]:

This leaks the stream object (an existing return also does).
Comment 2 Luis de Bethencourt 2016-08-01 15:05:07 UTC 
Created attachment 332479 [details] [review]
patch that fixes original memory leak

Thanks for the review Vincent. Sorry for missing the memory leak :S
Comment 3 Luis de Bethencourt 2016-08-01 15:05:47 UTC 
Created attachment 332480 [details] [review]
version 2 of patch to check return values

Second version
Comment 4 Luis de Bethencourt 2016-08-03 16:19:53 UTC 
Review of attachment 332479 [details] [review]:

commit 5cab7236510f01af42452cc269b19ee2bd44687b
Author: Luis de Bethencourt <luisbg@osg.samsung.com>
Date:   Mon Aug 1 15:52:11 2016 +0100

    ogg: fix memory leak in gst_ogg_parse_new_stream

    Avoid leaking the stream object

    https://bugzilla.gnome.org/show_bug.cgi?id=769299
Comment 5 Luis de Bethencourt 2016-08-03 16:20:18 UTC 
Review of attachment 332480 [details] [review]:

commit 7ae577dc3a4c1471e08260a9619b0dc2b42e4d7f
Author: Luis de Bethencourt <luisbg@osg.samsung.com>
Date:   Mon Aug 1 16:00:29 2016 +0100

    ogg: check return values in gst_ogg_parse_new_stream

    Return NULL in gst_ogg_parse_new_stream when either ogg_stream_pagein() or
    gst_ogg_stream_setup_map() failed.

    https://bugzilla.gnome.org/show_bug.cgi?id=769299