fix refcounting of warning and error messages
authorThomas Vander Stichele <thomas@apestaart.org>
Tue, 12 Jul 2005 16:28:36 +0000 (16:28 +0000)
committerThomas Vander Stichele <thomas@apestaart.org>
Tue, 12 Jul 2005 16:28:36 +0000 (16:28 +0000)
Original commit message from CVS:
fix refcounting of warning and error messages

ChangeLog
check/gst/gstelement.c
check/gst/gstmessage.c
check/gst/gststructure.c
gst/gstelement.c
gst/gstmessage.c
tests/check/gst/gstelement.c
tests/check/gst/gstmessage.c
tests/check/gst/gststructure.c

index eb77308..b57afcd 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
 2005-07-12  Thomas Vander Stichele  <thomas at apestaart dot org>
 
+       * check/gst/gstelement.c: (GST_START_TEST), (gst_element_suite):
+       * check/gst/gstmessage.c: (GST_START_TEST):
+       * check/gst/gststructure.c: (GST_START_TEST),
+       (gst_structure_suite), (main):
+         more testing
+       * gst/gstelement.c: (gst_element_message_full):
+         clean up GError and debug string now that they get copied
+       * gst/gstmessage.c: (gst_message_new_error),
+       (gst_message_new_warning), (gst_message_parse_error),
+       (gst_message_parse_warning):
+         use GST_TYPE_G_ERROR for structure_new, and take copies of
+         arguments, so that we don't mess up refcounting
+
+2005-07-12  Thomas Vander Stichele  <thomas at apestaart dot org>
+
        * check/Makefile.am:
          add per-test valgrind targets
        * check/gst-libs/gdp.c: (GST_START_TEST),
index e690fec..20e37ac 100644 (file)
@@ -78,6 +78,21 @@ GST_START_TEST (test_add_pad_unref_element)
 
 GST_END_TEST;
 
+GST_START_TEST (test_error_no_bus)
+{
+  GstElement *e;
+
+  e = gst_element_factory_make ("fakesrc", "source");
+
+  /* I don't want errors shown */
+  gst_debug_set_default_threshold (GST_LEVEL_NONE);
+
+  GST_ELEMENT_ERROR (e, RESOURCE, OPEN_READ, ("I could not read"), ("debug"));
+
+  gst_object_unref (e);
+}
+
+GST_END_TEST;
 
 Suite *
 gst_element_suite (void)
@@ -88,6 +103,7 @@ gst_element_suite (void)
   suite_add_tcase (s, tc_chain);
   tcase_add_test (tc_chain, test_add_remove_pad);
   tcase_add_test (tc_chain, test_add_pad_unref_element);
+  tcase_add_test (tc_chain, test_error_no_bus);
 
   return s;
 }
index a55a6ba..1d6c0c5 100644 (file)
@@ -49,8 +49,11 @@ GST_START_TEST (test_parsing)
     fail_if (message == NULL);
     fail_unless (GST_MESSAGE_TYPE (message) == GST_MESSAGE_ERROR);
     fail_unless (GST_MESSAGE_SRC (message) == NULL);
+
+    g_error_free (error);
     error = NULL;
     debug = NULL;
+
     gst_message_parse_error (message, &error, &debug);
     fail_if (error == NULL);
     fail_if (debug == NULL);
@@ -58,7 +61,10 @@ GST_START_TEST (test_parsing)
     fail_unless (error->domain == domain);
     fail_unless (error->code == 10);
     fail_unless (strcmp (debug, "error string") == 0);
+
     gst_message_unref (message);
+    g_error_free (error);
+    g_free (debug);
   }
   /* GST_MESSAGE_WARNING   */
   {
@@ -71,8 +77,11 @@ GST_START_TEST (test_parsing)
     fail_if (message == NULL);
     fail_unless (GST_MESSAGE_TYPE (message) == GST_MESSAGE_WARNING);
     fail_unless (GST_MESSAGE_SRC (message) == NULL);
+
+    g_error_free (warning);
     warning = NULL;
     debug = NULL;
+
     gst_message_parse_warning (message, &warning, &debug);
     fail_if (warning == NULL);
     fail_if (debug == NULL);
@@ -80,7 +89,10 @@ GST_START_TEST (test_parsing)
     fail_unless (warning->domain == domain);
     fail_unless (warning->code == 10);
     fail_unless (strcmp (debug, "warning string") == 0);
+
     gst_message_unref (message);
+    g_error_free (warning);
+    g_free (debug);
   }
   /* GST_MESSAGE_INFO   */
   {
@@ -101,6 +113,7 @@ GST_START_TEST (test_parsing)
     fail_if (tag == NULL);
     /* FIXME, check the actual tags */
     gst_message_unref (message);
+    gst_tag_list_free (tag);
   }
   /* GST_MESSAGE_BUFFERING   */
   {
index 06a1102..cf9eeeb 100644 (file)
@@ -79,12 +79,43 @@ gst_value_suite (void)
   return s;
 }
 
+GST_START_TEST (test_structure_new)
+{
+  GstStructure *s;
+  GError *e;
+  GQuark domain;
+
+  s = gst_structure_new ("name", "key", G_TYPE_STRING, "value", NULL);
+  fail_unless (strcmp (gst_structure_get_string (s, "key"), "value") == 0);
+  gst_structure_free (s);
+
+  domain = g_quark_from_string ("test");
+  e = g_error_new (domain, 0, "a test error");
+  s = gst_structure_new ("name", "key", GST_TYPE_G_ERROR, e, NULL);
+  g_error_free (e);
+  gst_structure_free (s);
+}
+
+GST_END_TEST;
+
+Suite *
+gst_structure_suite (void)
+{
+  Suite *s = suite_create ("GstStructure");
+  TCase *tc_chain = tcase_create ("general");
+
+  suite_add_tcase (s, tc_chain);
+  tcase_add_test (tc_chain, test_from_string_int);
+  tcase_add_test (tc_chain, test_structure_new);
+  return s;
+}
+
 int
 main (int argc, char **argv)
 {
   int nf;
 
-  Suite *s = gst_value_suite ();
+  Suite *s = gst_structure_suite ();
   SRunner *sr = srunner_create (s);
 
   gst_check_init (&argc, &argv);
index b274e51..544da34 100644 (file)
@@ -1356,9 +1356,11 @@ void gst_element_message_full
 
   if (type == GST_MESSAGE_ERROR) {
     message = gst_message_new_error (GST_OBJECT (element), gerror, sent_debug);
-  } else {
+  } else if (type == GST_MESSAGE_WARNING) {
     message = gst_message_new_warning (GST_OBJECT (element), gerror,
         sent_debug);
+  } else {
+    g_assert_not_reached ();
   }
   gst_element_post_message (element, message);
 
@@ -1366,8 +1368,9 @@ void gst_element_message_full
       sent_text);
 
   /* cleanup */
+  g_error_free (gerror);
+  g_free (sent_debug);
   g_free (sent_text);
-  /* sent_debug is not part of the gerror, so don't free it here */
 }
 
 /**
index 7bdf166..ce0402f 100644 (file)
@@ -23,6 +23,7 @@
 #include <string.h>             /* memcpy */
 
 #include "gst_private.h"
+#include "gsterror.h"
 #include "gstinfo.h"
 #include "gstmemchunk.h"
 #include "gstmessage.h"
@@ -206,7 +207,7 @@ gst_message_new_eos (GstObject * src)
  * @error: The GError for this message.
  * @debug: A debugging string for something or other.
  *
- * Create a new error message. The message will take ownership of @error and
+ * Create a new error message. The message will copy @error and
  * @debug.
  *
  * Returns: The new error message.
@@ -220,7 +221,8 @@ gst_message_new_error (GstObject * src, GError * error, gchar * debug)
   GstStructure *s;
 
   message = gst_message_new (GST_MESSAGE_ERROR, src);
-  s = gst_structure_new ("GstMessageError", "gerror", G_TYPE_POINTER, error,
+  /* gst_structure_new takes copies of the types passed in */
+  s = gst_structure_new ("GstMessageError", "gerror", GST_TYPE_G_ERROR, error,
       "debug", G_TYPE_STRING, debug, NULL);
   gst_structure_set_parent_refcount (s, &message->mini_object.refcount);
   message->structure = s;
@@ -234,7 +236,7 @@ gst_message_new_error (GstObject * src, GError * error, gchar * debug)
  * @error: The GError for this message.
  * @debug: A debugging string for something or other.
  *
- * Create a new warning message. The message will take ownership of @error and
+ * Create a new warning message. The message will make copies of @error and
  * @debug.
  *
  * Returns: The new warning message.
@@ -248,7 +250,8 @@ gst_message_new_warning (GstObject * src, GError * error, gchar * debug)
   GstStructure *s;
 
   message = gst_message_new (GST_MESSAGE_WARNING, src);
-  s = gst_structure_new ("GstMessageWarning", "gerror", G_TYPE_POINTER, error,
+  /* gst_structure_new takes copies of the types passed in */
+  s = gst_structure_new ("GstMessageWarning", "gerror", GST_TYPE_G_ERROR, error,
       "debug", G_TYPE_STRING, debug, NULL);
   gst_structure_set_parent_refcount (s, &message->mini_object.refcount);
   message->structure = s;
@@ -416,9 +419,9 @@ gst_message_parse_error (GstMessage * message, GError ** gerror, gchar ** debug)
 
   error_gvalue = gst_structure_get_value (message->structure, "gerror");
   g_return_if_fail (error_gvalue != NULL);
-  g_return_if_fail (G_VALUE_TYPE (error_gvalue) == G_TYPE_POINTER);
+  g_return_if_fail (G_VALUE_TYPE (error_gvalue) == GST_TYPE_G_ERROR);
 
-  error_val = g_value_get_pointer (error_gvalue);
+  error_val = (GError *) g_value_get_boxed (error_gvalue);
   if (error_val)
     *gerror = g_error_copy (error_val);
   else
@@ -447,9 +450,9 @@ gst_message_parse_warning (GstMessage * message, GError ** gerror,
 
   error_gvalue = gst_structure_get_value (message->structure, "gerror");
   g_return_if_fail (error_gvalue != NULL);
-  g_return_if_fail (G_VALUE_TYPE (error_gvalue) == G_TYPE_POINTER);
+  g_return_if_fail (G_VALUE_TYPE (error_gvalue) == GST_TYPE_G_ERROR);
 
-  error_val = g_value_get_pointer (error_gvalue);
+  error_val = (GError *) g_value_get_boxed (error_gvalue);
   if (error_val)
     *gerror = g_error_copy (error_val);
   else
index e690fec..20e37ac 100644 (file)
@@ -78,6 +78,21 @@ GST_START_TEST (test_add_pad_unref_element)
 
 GST_END_TEST;
 
+GST_START_TEST (test_error_no_bus)
+{
+  GstElement *e;
+
+  e = gst_element_factory_make ("fakesrc", "source");
+
+  /* I don't want errors shown */
+  gst_debug_set_default_threshold (GST_LEVEL_NONE);
+
+  GST_ELEMENT_ERROR (e, RESOURCE, OPEN_READ, ("I could not read"), ("debug"));
+
+  gst_object_unref (e);
+}
+
+GST_END_TEST;
 
 Suite *
 gst_element_suite (void)
@@ -88,6 +103,7 @@ gst_element_suite (void)
   suite_add_tcase (s, tc_chain);
   tcase_add_test (tc_chain, test_add_remove_pad);
   tcase_add_test (tc_chain, test_add_pad_unref_element);
+  tcase_add_test (tc_chain, test_error_no_bus);
 
   return s;
 }
index a55a6ba..1d6c0c5 100644 (file)
@@ -49,8 +49,11 @@ GST_START_TEST (test_parsing)
     fail_if (message == NULL);
     fail_unless (GST_MESSAGE_TYPE (message) == GST_MESSAGE_ERROR);
     fail_unless (GST_MESSAGE_SRC (message) == NULL);
+
+    g_error_free (error);
     error = NULL;
     debug = NULL;
+
     gst_message_parse_error (message, &error, &debug);
     fail_if (error == NULL);
     fail_if (debug == NULL);
@@ -58,7 +61,10 @@ GST_START_TEST (test_parsing)
     fail_unless (error->domain == domain);
     fail_unless (error->code == 10);
     fail_unless (strcmp (debug, "error string") == 0);
+
     gst_message_unref (message);
+    g_error_free (error);
+    g_free (debug);
   }
   /* GST_MESSAGE_WARNING   */
   {
@@ -71,8 +77,11 @@ GST_START_TEST (test_parsing)
     fail_if (message == NULL);
     fail_unless (GST_MESSAGE_TYPE (message) == GST_MESSAGE_WARNING);
     fail_unless (GST_MESSAGE_SRC (message) == NULL);
+
+    g_error_free (warning);
     warning = NULL;
     debug = NULL;
+
     gst_message_parse_warning (message, &warning, &debug);
     fail_if (warning == NULL);
     fail_if (debug == NULL);
@@ -80,7 +89,10 @@ GST_START_TEST (test_parsing)
     fail_unless (warning->domain == domain);
     fail_unless (warning->code == 10);
     fail_unless (strcmp (debug, "warning string") == 0);
+
     gst_message_unref (message);
+    g_error_free (warning);
+    g_free (debug);
   }
   /* GST_MESSAGE_INFO   */
   {
@@ -101,6 +113,7 @@ GST_START_TEST (test_parsing)
     fail_if (tag == NULL);
     /* FIXME, check the actual tags */
     gst_message_unref (message);
+    gst_tag_list_free (tag);
   }
   /* GST_MESSAGE_BUFFERING   */
   {
index 06a1102..cf9eeeb 100644 (file)
@@ -79,12 +79,43 @@ gst_value_suite (void)
   return s;
 }
 
+GST_START_TEST (test_structure_new)
+{
+  GstStructure *s;
+  GError *e;
+  GQuark domain;
+
+  s = gst_structure_new ("name", "key", G_TYPE_STRING, "value", NULL);
+  fail_unless (strcmp (gst_structure_get_string (s, "key"), "value") == 0);
+  gst_structure_free (s);
+
+  domain = g_quark_from_string ("test");
+  e = g_error_new (domain, 0, "a test error");
+  s = gst_structure_new ("name", "key", GST_TYPE_G_ERROR, e, NULL);
+  g_error_free (e);
+  gst_structure_free (s);
+}
+
+GST_END_TEST;
+
+Suite *
+gst_structure_suite (void)
+{
+  Suite *s = suite_create ("GstStructure");
+  TCase *tc_chain = tcase_create ("general");
+
+  suite_add_tcase (s, tc_chain);
+  tcase_add_test (tc_chain, test_from_string_int);
+  tcase_add_test (tc_chain, test_structure_new);
+  return s;
+}
+
 int
 main (int argc, char **argv)
 {
   int nf;
 
-  Suite *s = gst_value_suite ();
+  Suite *s = gst_structure_suite ();
   SRunner *sr = srunner_create (s);
 
   gst_check_init (&argc, &argv);