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 746810 - matroska: fix GValue leak when parsing tags
matroska: fix GValue leak when parsing tags
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-26 12:34 UTC by Guillaume Desmottes
Modified: 2015-03-30 12:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
matroska: fix GValue leaks when parsing tags (3.28 KB, patch)
2015-03-26 12:35 UTC, Guillaume Desmottes
none Details | Review
matroska: fix GValue leaks when parsing tags (4.00 KB, patch)
2015-03-30 07:29 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2015-03-26 12:34:21 UTC
.
Comment 1 Guillaume Desmottes 2015-03-26 12:35:53 UTC
Created attachment 300350 [details] [review]
matroska: fix GValue leaks when parsing tags

gst_tag_list_add_value() doesn't consume the GValue we pass to it so we should
always unset it after calling it.
Comment 2 Thiago Sousa Santos 2015-03-27 12:45:16 UTC
Review of attachment 300350 [details] [review]:

If the value is copied internally by the gsttaglist, then I guess there is no need for this gvalue to be copied in this function. We can just pass the original value and let taglist copy it inside to avoid double copying. What do you think?
Comment 3 Guillaume Desmottes 2015-03-27 13:36:38 UTC
Yeah I considered that as well but was wondering if the value was copied for another reason that I missed.
Comment 4 Thiago Sousa Santos 2015-03-27 14:32:44 UTC
I don't see any other reason. GstTagList seems to always copy it internally in the same way: g_value_init and g_value_copy.

I assume it would be ok just to pass the original value and let the taglist copy it.
Comment 5 Guillaume Desmottes 2015-03-30 07:29:18 UTC
Created attachment 300563 [details] [review]
matroska: fix GValue leaks when parsing tags

gst_tag_list_add_value() doesn't consume the GValue we pass to it so there is
no point copying it.
Comment 6 Thiago Sousa Santos 2015-03-30 12:09:14 UTC
Thanks for the update

commit 592cab1512b52142f10f144cfdbb0abba40e924b
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Thu Mar 26 13:34:53 2015 +0100

    matroska: fix GValue leaks when parsing tags
    
    gst_tag_list_add_value() doesn't consume the GValue we pass to it so there is
    no point copying it.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=746810