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 751823 - glimagesink: possible null pointer dereference
glimagesink: possible null pointer dereference
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other Linux
: Normal minor
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-07-02 07:37 UTC by Vineeth
Modified: 2015-08-16 13:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
prevent possible null pointer dereference (1.27 KB, patch)
2015-07-02 07:38 UTC, Vineeth
none Details | Review
use g_clear_error (833 bytes, patch)
2015-07-02 08:09 UTC, Vineeth
committed Details | Review

Description Vineeth 2015-07-02 07:37:56 UTC
during context_error, while trying to print the error message,
 there is no check if error is allocated. Ideally error will be allocated
 and we need not have a check at all. But just in case
Comment 1 Vineeth 2015-07-02 07:38:53 UTC
Created attachment 306588 [details] [review]
prevent possible null pointer dereference
Comment 2 Tim-Philipp Müller 2015-07-02 07:57:17 UTC
I see where you're coming from here (I'm guessing some static analyzer complained that you're dereferencing error unconditionally and later check if it's non-NULL), but I don't think this should be fixed like that.

If a function returns a GError on failure, it should always do that. And _create_context() always does that as far as I can tell. So the right thing to do would be to drop the if (error) check here, or to use g_clear_error() instead (which may or may not make the static analyzer shut up).
Comment 3 Vineeth 2015-07-02 08:09:55 UTC
Created attachment 306590 [details] [review]
use g_clear_error

g_clear_error does the trick of shutting up static analysis tool :)
Comment 4 Luis de Bethencourt 2015-07-02 10:31:40 UTC
Thanks Vineeth, and good explanation in the commit message.

"replace g_error_free with g_clear_error, as it internally checks if error variable is valid or not."
Comment 5 Luis de Bethencourt 2015-07-02 10:35:19 UTC
Review of attachment 306590 [details] [review]:

commit 8ec2e0ad6212865aeb76cb30a3fd716a081cbced
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Thu Jul 2 17:08:26 2015 +0900

    glimagesink: use g_clear_error instead of g_error_free

    replace g_error_free with g_clear_error, as it internally
    checks if error variable is valid or not.

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