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 753340 - validate: media-check: Handle NULL GError in media descriptor writer
validate: media-check: Handle NULL GError in media descriptor writer
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-devtools
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-08-07 04:02 UTC by Vineeth
Modified: 2015-10-02 15:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix memory leak (1.36 KB, patch)
2015-08-07 04:04 UTC, Vineeth
none Details | Review
Handle NULL GError parameter (1.92 KB, patch)
2015-08-17 01:43 UTC, Vineeth
none Details | Review
If not using GError, pass NULL instead (1.35 KB, patch)
2015-08-17 01:43 UTC, Vineeth
committed Details | Review
Handle NULL GError parameter (1.80 KB, patch)
2015-08-18 23:48 UTC, Vineeth
committed Details | Review

Description Vineeth 2015-08-07 04:02:24 UTC
err variable being passed to descriptor_writer and descriptor_parser, will get 
allocated memory incase of error scenarios. But the same is not getting freed
Comment 1 Vineeth 2015-08-07 04:04:07 UTC
Created attachment 308874 [details] [review]
fix memory leak
Comment 2 Tim-Philipp Müller 2015-08-13 12:56:12 UTC
Comment on attachment 308874 [details] [review]
fix memory leak

Thanks for the patch.

This doesn't look like the right GError use pattern to me.

If code is not interested in the GError, it should just pass NULL instead of a GError address (if the API in question doesn't accept NULL for the GError address, that should be fixed).

Next, the GError should probably be freed in the code path where the failure occured (we can/should assume that the GError is set in that case).
Comment 3 Vineeth 2015-08-17 01:43:12 UTC
Created attachment 309384 [details] [review]
Handle NULL GError parameter

Media descriptor writer should be able to handle NULL GError parameter, and free the error variable in case of failures.
Comment 4 Vineeth 2015-08-17 01:43:36 UTC
Created attachment 309385 [details] [review]
If not using GError, pass NULL instead
Comment 5 Vineeth 2015-08-18 23:48:02 UTC
Created attachment 309513 [details] [review]
Handle NULL GError parameter

as per changes in https://bugzilla.gnome.org/show_bug.cgi?id=753701
now when info is NULL, err will be filled. So there is no need to check for presence of info.
And if NULL GError is being passed from the caller, freeing the GError else propagating the same.
Comment 6 Thibault Saunier 2015-10-02 15:40:07 UTC
commit e1c1c45eb82e79915f46bcbcf113f3287eefbed3
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Mon Aug 17 10:40:22 2015 +0900

    validate: media-check: Pass NULL instead of GError if not using it
    
    If not using the GError being passed on to media descriptor, writer and parser,
    simply pass NULL instead of GError.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=753340

commit 15c87003b8c48a473b1160a5d93e12b06f3fe90c
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Mon Aug 17 10:31:33 2015 +0900

    validate: descriptor-writer: Handle NULL GError address and free GError during error cases
    
    writer_new_discover() API should be able to accept NULL GError and in case of
    error, if GError is passed on as parameter, it should be propagated, else it
    should be free'd.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=753340