From dd80b2030c57f3b9394f367ae4de2f78433cb20f Mon Sep 17 00:00:00 2001 From: Edward Hervey Date: Sat, 30 Aug 2008 11:55:59 +0000 Subject: [PATCH] Fix all leaks due to the bug in gst_pad_template_new() by which it does not steal the refcount of the given caps as s... Original commit message from CVS: * gst/gstutils.c: (gst_element_get_compatible_pad): * tests/check/gst/gstghostpad.c: (GST_START_TEST): * tests/check/gst/gstpad.c: (name_is_valid), (GST_START_TEST): Fix all leaks due to the bug in gst_pad_template_new() by which it does not steal the refcount of the given caps as stated. REVERT THIS COMMIT ONCE FIXED ! REVERT THIS COMMIT ONCE FIXED ! REVERT THIS COMMIT ONCE FIXED ! REVERT THIS COMMIT ONCE FIXED ! REVERT THIS COMMIT ONCE FIXED ! REVERT THIS COMMIT ONCE FIXED ! --- ChangeLog | 15 ++++++++++++++ gst/gstutils.c | 6 ++++++ tests/check/gst/gstghostpad.c | 25 ++++++++++++++++++++--- tests/check/gst/gstpad.c | 37 +++++++++++++++++++++++++++-------- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index 48e2bc2979..1c5002a6cc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2008-08-30 Edward Hervey + + * gst/gstutils.c: (gst_element_get_compatible_pad): + * tests/check/gst/gstghostpad.c: (GST_START_TEST): + * tests/check/gst/gstpad.c: (name_is_valid), (GST_START_TEST): + Fix all leaks due to the bug in gst_pad_template_new() by which it does + not steal the refcount of the given caps as stated. + + REVERT THIS COMMIT ONCE FIXED ! + REVERT THIS COMMIT ONCE FIXED ! + REVERT THIS COMMIT ONCE FIXED ! + REVERT THIS COMMIT ONCE FIXED ! + REVERT THIS COMMIT ONCE FIXED ! + REVERT THIS COMMIT ONCE FIXED ! + 2008-08-29 Wim Taymans * gst/gstiterator.c: diff --git a/gst/gstutils.c b/gst/gstutils.c index b28f6a1370..ec8fc44c11 100644 --- a/gst/gstutils.c +++ b/gst/gstutils.c @@ -969,6 +969,12 @@ gst_element_get_compatible_pad (GstElement * element, GstPad * pad, templ = gst_pad_template_new ((gchar *) GST_PAD_NAME (pad), GST_PAD_DIRECTION (pad), GST_PAD_ALWAYS, templcaps); + + /* FIXME : Because of a bug in gst_pad_template_new() by which is does not + * properly steal the refcount of the given caps, we have to unref these caps + * REVERT THIS WHEN FIXED !*/ + gst_caps_unref (templcaps); + foundpad = gst_element_request_compatible_pad (element, templ); gst_object_unref (templ); diff --git a/tests/check/gst/gstghostpad.c b/tests/check/gst/gstghostpad.c index b5a28c9c08..577cc49954 100644 --- a/tests/check/gst/gstghostpad.c +++ b/tests/check/gst/gstghostpad.c @@ -558,18 +558,27 @@ GST_START_TEST (test_ghost_pads_new_from_template) GstPadTemplate *padtempl, *ghosttempl; GstCaps *padcaps, *ghostcaps, *newcaps; + GstCaps *copypadcaps; padcaps = gst_caps_from_string ("some/caps"); fail_unless (padcaps != NULL); ghostcaps = gst_caps_from_string ("some/caps;some/other-caps"); fail_unless (ghostcaps != NULL); + copypadcaps = gst_caps_copy (padcaps); padtempl = gst_pad_template_new ("padtempl", GST_PAD_SINK, - GST_PAD_ALWAYS, gst_caps_copy (padcaps)); + GST_PAD_ALWAYS, copypadcaps); fail_unless (padtempl != NULL); ghosttempl = gst_pad_template_new ("ghosttempl", GST_PAD_SINK, GST_PAD_ALWAYS, ghostcaps); + /* FIXME : We should not have to unref those caps, but due to + * a bug in gst_pad_template_new() not stealing the refcount of + * the given caps we have to. */ + gst_caps_unref (ghostcaps); + gst_caps_unref (copypadcaps); + + sinkpad = gst_pad_new_from_template (padtempl, "sinkpad"); fail_unless (sinkpad != NULL); @@ -602,17 +611,27 @@ GST_START_TEST (test_ghost_pads_new_no_target_from_template) GstPadTemplate *padtempl, *ghosttempl; GstCaps *padcaps, *ghostcaps, *newcaps; + GstCaps *copypadcaps, *copyghostcaps; padcaps = gst_caps_from_string ("some/caps"); fail_unless (padcaps != NULL); ghostcaps = gst_caps_from_string ("some/caps;some/other-caps"); fail_unless (ghostcaps != NULL); + copypadcaps = gst_caps_copy (padcaps); + copyghostcaps = gst_caps_copy (ghostcaps); + padtempl = gst_pad_template_new ("padtempl", GST_PAD_SINK, - GST_PAD_ALWAYS, gst_caps_copy (padcaps)); + GST_PAD_ALWAYS, copypadcaps); fail_unless (padtempl != NULL); ghosttempl = gst_pad_template_new ("ghosttempl", GST_PAD_SINK, - GST_PAD_ALWAYS, gst_caps_copy (ghostcaps)); + GST_PAD_ALWAYS, copyghostcaps); + + /* FIXME : We should not have to unref those caps, but due to + * a bug in gst_pad_template_new() not stealing the refcount of + * the given caps we have to. */ + gst_caps_unref (copyghostcaps); + gst_caps_unref (copypadcaps); sinkpad = gst_pad_new_from_template (padtempl, "sinkpad"); fail_unless (sinkpad != NULL); diff --git a/tests/check/gst/gstpad.c b/tests/check/gst/gstpad.c index 8c67a43d07..a3610e742b 100644 --- a/tests/check/gst/gstpad.c +++ b/tests/check/gst/gstpad.c @@ -203,10 +203,15 @@ static gboolean name_is_valid (const gchar * name, GstPadPresence presence) { GstPadTemplate *new; + GstCaps *any = GST_CAPS_ANY; - new = gst_pad_template_new (name, GST_PAD_SRC, presence, GST_CAPS_ANY); + new = gst_pad_template_new (name, GST_PAD_SRC, presence, any); if (new) { gst_object_unref (GST_OBJECT (new)); + /* FIXME : We should not have to unref those caps, but due to + * a bug in gst_pad_template_new() not stealing the refcount of + * the given caps we have to. */ + gst_caps_unref (any); return TRUE; } return FALSE; @@ -463,16 +468,26 @@ GST_START_TEST (test_push_negotiation) { GstPad *src, *sink; GstPadLinkReturn plr; - GstPadTemplate *src_template = gst_pad_template_new ("src", GST_PAD_SRC, - GST_PAD_ALWAYS, - gst_caps_from_string ("audio/x-raw-int,width={16,32},depth={16,32}")); - GstPadTemplate *sink_template = gst_pad_template_new ("sink", GST_PAD_SINK, - GST_PAD_ALWAYS, - gst_caps_from_string ("audio/x-raw-int,width=32,depth={16,32}")); + GstCaps *srccaps = + gst_caps_from_string ("audio/x-raw-int,width={16,32},depth={16,32}"); + GstCaps *sinkcaps = + gst_caps_from_string ("audio/x-raw-int,width=32,depth={16,32}"); + GstPadTemplate *src_template; + GstPadTemplate *sink_template; GstCaps *caps; GstBuffer *buffer; /* setup */ + src_template = gst_pad_template_new ("src", GST_PAD_SRC, + GST_PAD_ALWAYS, srccaps); + sink_template = gst_pad_template_new ("sink", GST_PAD_SINK, + GST_PAD_ALWAYS, sinkcaps); + /* FIXME : We should not have to unref those caps, but due to + * a bug in gst_pad_template_new() not stealing the refcount of + * the given caps we have to. */ + gst_caps_unref (srccaps); + gst_caps_unref (sinkcaps); + sink = gst_pad_new_from_template (sink_template, "sink"); fail_if (sink == NULL); gst_pad_set_chain_function (sink, gst_check_chain_func); @@ -588,17 +603,23 @@ GST_START_TEST (test_get_caps_must_be_copy) caps = gst_caps_new_any (); templ = gst_pad_template_new ("test_templ", GST_PAD_SRC, GST_PAD_ALWAYS, caps); + /* FIXME : This is not correct behaviour, but due to a bug with + * gst_pad_template_new() not stealing the refcount of the given caps, + * we need to unref it */ + gst_caps_unref (caps); + pad = gst_pad_new_from_template (templ, NULL); fail_unless (GST_PAD_CAPS (pad) == NULL, "caps present on pad"); + /* This is a writable copy ! */ caps = gst_pad_get_caps (pad); /* we must own the caps */ ASSERT_OBJECT_REFCOUNT (caps, "caps", 1); /* cleanup */ + gst_object_unref (templ); gst_caps_unref (caps); gst_object_unref (pad); - gst_object_unref (templ); } GST_END_TEST;