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 753088 - decodebin: fix deadend_details string leak
decodebin: fix deadend_details string leak
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-07-31 04:31 UTC by Vineeth
Modified: 2015-08-16 13:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix string leak (881 bytes, patch)
2015-07-31 04:33 UTC, Vineeth
none Details | Review
fix string leak (1007 bytes, patch)
2015-07-31 07:42 UTC, Vineeth
none Details | Review

Description Vineeth 2015-07-31 04:31:50 UTC
deadend_details is being to the chain value only when ret of connect_pad is false, but when ret is true, it is not being free'd leading to leak.
Comment 1 Vineeth 2015-07-31 04:33:52 UTC
Created attachment 308508 [details] [review]
fix string leak
Comment 2 Thiago Sousa Santos 2015-07-31 05:45:15 UTC
Review of attachment 308508 [details] [review]:

There seems to be a typo or some missing word in the commit message.

Is there an easy way to reproduce the leak? I guess it happens when a first try at linking an element fails but a second element succeeds, so the previous element errors are still returned.

Would it make sense to have the connect_pad clean up the variable itself in case the pad is not a deadend? It seems weird to return deadend_details if the pad is not a deadend.

::: gst/playback/gstdecodebin2.c
@@ +1788,1 @@
 

g_free already does NULL check internally, you can simply call g_free (deadend_details) here.
Comment 3 Vineeth 2015-07-31 07:42:56 UTC
Created attachment 308513 [details] [review]
fix string leak

As suggested by Thiago, 
connect pad should not pass the deadend details in case it is not a dead end.
Hence checking for res value and freeing if it is TRUE.
Comment 4 Thiago Sousa Santos 2015-08-05 17:38:51 UTC
Simplified it by just doing "|| res" instead of "|| res == TRUE" as res is a boolean.

commit 0a3fe31f26d254a9570eb01f8e61468ba3014d53
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Fri Jul 31 13:31:56 2015 +0900

    decodebin: fix deadend_details string leak
    
    deadend_details need not be returned when the pad is not a deadend.
    Hence checking if res value is TRUE and clearing the string instead of
    passing it on
    
    https://bugzilla.gnome.org/show_bug.cgi?id=753088