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

@@ -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,
+    g_warning ("no property %s in object %s", name, GST_OBJECT_NAME (object));
-        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) {
+    if (error)
-      g_warning ("error copying value: %s", error);
+      goto cant_copy;
-      return;
-    }
     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;
+    if (!gst_child_proxy_lookup (object, name, &target, &pspec))
-    GstObject *target;
+      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) {
+    if (error)
-      g_warning ("error copying value: %s", error);
+      goto cant_copy;
-      gst_object_unref (target);
+
-      return;
-    }
     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;
+  }
 }
 
 /**

tests/check/Makefile.am

@@ -46,6 +46,7 @@ else
 REGISTRY_CHECKS =				\
 	gst/gst					\
 	gst/gstbin				\
+	gst/gstchildproxy			\
 	gst/gstelement				\
 	gst/gstevent				\
 	gst/gstghostpad				\

tests/check/gst/.gitignore

@@ -6,6 +6,7 @@ gstbuffer
 gstbufferlist
 gstbus
 gstcaps
+gstchildproxy
 gstdata
 gstelement
 gstevent

tests/check/gst/gstchildproxy.c

@@ -0,0 +1,77 @@
+/* GStreamer
+ * Copyright (C) 2009 Stefan Kost <ensonic@users.sf.net>
+ *
+ * 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/check/gstcheck.h>
+
+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);