From 09c7d349210763d0ee295eda032023c00d97c8d3 Mon Sep 17 00:00:00 2001 From: Stefan Kost Date: Mon, 5 Oct 2009 11:46:34 +0300 Subject: [PATCH] childproxy: initialize gvalue in _valist function. Fixes #595602 Reflow the code to move error handling to the end of the functions. Initialize gvalue like we do in the setter. Add a unit-test module with two simple tests the catche this bug. --- gst/gstchildproxy.c | 67 ++++++++++++++++++++-------- tests/check/Makefile.am | 1 + tests/check/gst/.gitignore | 1 + tests/check/gst/gstchildproxy.c | 77 +++++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 19 deletions(-) create mode 100644 tests/check/gst/gstchildproxy.c diff --git a/gst/gstchildproxy.c b/gst/gstchildproxy.c index bb54e294ae..b2ca36c5d2 100644 --- a/gst/gstchildproxy.c +++ b/gst/gstchildproxy.c @@ -249,8 +249,7 @@ gst_child_proxy_get_property (GstObject * object, const gchar * name, not_found: { - g_warning ("cannot get property %s from object %s", name, - GST_OBJECT_NAME (object)); + g_warning ("no property %s in object %s", name, GST_OBJECT_NAME (object)); return; } } @@ -270,6 +269,8 @@ gst_child_proxy_get_valist (GstObject * object, const gchar *name; gchar *error = NULL; GValue value = { 0, }; + GParamSpec *pspec; + GstObject *target; g_return_if_fail (G_IS_OBJECT (object)); @@ -277,15 +278,33 @@ gst_child_proxy_get_valist (GstObject * object, /* iterate over pairs */ while (name) { - gst_child_proxy_get_property (object, name, &value); + if (!gst_child_proxy_lookup (object, name, &target, &pspec)) + goto not_found; + + g_value_init (&value, G_PARAM_SPEC_VALUE_TYPE (pspec)); + g_object_get_property (G_OBJECT (target), pspec->name, &value); + gst_object_unref (target); + G_VALUE_LCOPY (&value, var_args, 0, &error); - if (error) { - g_warning ("error copying value: %s", error); - return; - } + if (error) + goto cant_copy; g_value_unset (&value); name = va_arg (var_args, gchar *); } + return; + +not_found: + { + g_warning ("no property %s in object %s", name, GST_OBJECT_NAME (object)); + return; + } +cant_copy: + { + g_warning ("error copying value %s in object %s: %s", pspec->name, + GST_OBJECT_NAME (object), error); + g_value_unset (&value); + return; + } } /** @@ -357,6 +376,8 @@ gst_child_proxy_set_valist (GstObject * object, const gchar *name; gchar *error = NULL; GValue value = { 0, }; + GParamSpec *pspec; + GstObject *target; g_return_if_fail (G_IS_OBJECT (object)); @@ -364,27 +385,35 @@ gst_child_proxy_set_valist (GstObject * object, /* iterate over pairs */ while (name) { - GParamSpec *pspec; - GstObject *target; + if (!gst_child_proxy_lookup (object, name, &target, &pspec)) + goto not_found; - if (!gst_child_proxy_lookup (object, name, &target, &pspec)) { - g_warning ("no such property %s in object %s", name, - GST_OBJECT_NAME (object)); - continue; - } g_value_init (&value, G_PARAM_SPEC_VALUE_TYPE (pspec)); G_VALUE_COLLECT (&value, var_args, G_VALUE_NOCOPY_CONTENTS, &error); - if (error) { - g_warning ("error copying value: %s", error); - gst_object_unref (target); - return; - } + if (error) + goto cant_copy; + g_object_set_property (G_OBJECT (target), pspec->name, &value); gst_object_unref (target); g_value_unset (&value); name = va_arg (var_args, gchar *); } + return; + +not_found: + { + g_warning ("no property %s in object %s", name, GST_OBJECT_NAME (object)); + return; + } +cant_copy: + { + g_warning ("error copying value %s in object %s: %s", pspec->name, + GST_OBJECT_NAME (object), error); + g_value_unset (&value); + gst_object_unref (target); + return; + } } /** diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index 3437792dbf..88942f80be 100644 --- a/tests/check/Makefile.am +++ b/tests/check/Makefile.am @@ -46,6 +46,7 @@ else REGISTRY_CHECKS = \ gst/gst \ gst/gstbin \ + gst/gstchildproxy \ gst/gstelement \ gst/gstevent \ gst/gstghostpad \ diff --git a/tests/check/gst/.gitignore b/tests/check/gst/.gitignore index 7f9374fa2d..783748b993 100644 --- a/tests/check/gst/.gitignore +++ b/tests/check/gst/.gitignore @@ -6,6 +6,7 @@ gstbuffer gstbufferlist gstbus gstcaps +gstchildproxy gstdata gstelement gstevent diff --git a/tests/check/gst/gstchildproxy.c b/tests/check/gst/gstchildproxy.c new file mode 100644 index 0000000000..43370ff8db --- /dev/null +++ b/tests/check/gst/gstchildproxy.c @@ -0,0 +1,77 @@ +/* GStreamer + * Copyright (C) 2009 Stefan Kost + * + * gstchildproxy.c: Unit test for GstChildProxy interface + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Library General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Library General Public License for more details. + * + * You should have received a copy of the GNU Library General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#include + +GST_START_TEST (test_get) +{ + GstElement *pipeline; + gchar *name; + + pipeline = gst_pipeline_new ("foo"); + fail_unless (pipeline != NULL, "Could not create pipeline"); + + gst_child_proxy_get (GST_OBJECT (pipeline), "name", &name, NULL); + fail_if (g_strcmp0 ("foo", name)); + + gst_object_unref (pipeline); +} + +GST_END_TEST; + +GST_START_TEST (test_child_get) +{ + GstElement *pipeline, *elem; + gchar *name; + + pipeline = gst_pipeline_new (NULL); + fail_unless (pipeline != NULL, "Could not create pipeline"); + + elem = gst_element_factory_make ("fakesrc", "src"); + fail_if (elem == NULL, "Could not create fakesrc"); + + gst_bin_add (GST_BIN (pipeline), elem); + + gst_child_proxy_get (GST_OBJECT (pipeline), "src::name", &name, NULL); + fail_if (g_strcmp0 ("src", name)); + + gst_object_unref (pipeline); +} + +GST_END_TEST; + + +static Suite * +gst_child_proxy_suite (void) +{ + Suite *s = suite_create ("GstChildProxy"); + TCase *tc_chain = tcase_create ("child proxy tests"); + + tcase_set_timeout (tc_chain, 0); + + suite_add_tcase (s, tc_chain); + tcase_add_test (tc_chain, test_get); + tcase_add_test (tc_chain, test_child_get); + + return s; +} + +GST_CHECK_MAIN (gst_child_proxy);