From 232b0ba690c1cc256eea71d49c67971c3cc68fde Mon Sep 17 00:00:00 2001 From: Thomas Vander Stichele Date: Tue, 12 Jul 2005 16:28:36 +0000 Subject: [PATCH] fix refcounting of warning and error messages Original commit message from CVS: fix refcounting of warning and error messages --- ChangeLog | 15 +++++++++++++++ check/gst/gstelement.c | 16 ++++++++++++++++ check/gst/gstmessage.c | 13 +++++++++++++ check/gst/gststructure.c | 33 ++++++++++++++++++++++++++++++++- gst/gstelement.c | 7 +++++-- gst/gstmessage.c | 19 +++++++++++-------- tests/check/gst/gstelement.c | 16 ++++++++++++++++ tests/check/gst/gstmessage.c | 13 +++++++++++++ tests/check/gst/gststructure.c | 33 ++++++++++++++++++++++++++++++++- 9 files changed, 153 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index eb77308..b57afcd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,20 @@ 2005-07-12 Thomas Vander Stichele + * 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 + * check/Makefile.am: add per-test valgrind targets * check/gst-libs/gdp.c: (GST_START_TEST), diff --git a/check/gst/gstelement.c b/check/gst/gstelement.c index e690fec..20e37ac 100644 --- a/check/gst/gstelement.c +++ b/check/gst/gstelement.c @@ -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; } diff --git a/check/gst/gstmessage.c b/check/gst/gstmessage.c index a55a6ba..1d6c0c5 100644 --- a/check/gst/gstmessage.c +++ b/check/gst/gstmessage.c @@ -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 */ { diff --git a/check/gst/gststructure.c b/check/gst/gststructure.c index 06a1102..cf9eeeb 100644 --- a/check/gst/gststructure.c +++ b/check/gst/gststructure.c @@ -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); diff --git a/gst/gstelement.c b/gst/gstelement.c index b274e51..544da34 100644 --- a/gst/gstelement.c +++ b/gst/gstelement.c @@ -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 */ } /** diff --git a/gst/gstmessage.c b/gst/gstmessage.c index 7bdf166..ce0402f 100644 --- a/gst/gstmessage.c +++ b/gst/gstmessage.c @@ -23,6 +23,7 @@ #include /* 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 diff --git a/tests/check/gst/gstelement.c b/tests/check/gst/gstelement.c index e690fec..20e37ac 100644 --- a/tests/check/gst/gstelement.c +++ b/tests/check/gst/gstelement.c @@ -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; } diff --git a/tests/check/gst/gstmessage.c b/tests/check/gst/gstmessage.c index a55a6ba..1d6c0c5 100644 --- a/tests/check/gst/gstmessage.c +++ b/tests/check/gst/gstmessage.c @@ -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 */ { diff --git a/tests/check/gst/gststructure.c b/tests/check/gst/gststructure.c index 06a1102..cf9eeeb 100644 --- a/tests/check/gst/gststructure.c +++ b/tests/check/gst/gststructure.c @@ -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); -- 2.7.4