diff --git a/gst/gststructure.c b/gst/gststructure.c index 2d512b6ea3..77ebc49d24 100644 --- a/gst/gststructure.c +++ b/gst/gststructure.c @@ -657,18 +657,19 @@ gst_structure_set_field (GstStructure * structure, GstStructureField * field) s = g_value_get_string (&field->value); /* only check for NULL strings in taglists, as they are allowed in message * structs, e.g. error message debug strings */ - if (G_UNLIKELY (s == NULL && IS_TAGLIST (structure))) { - g_warning ("Trying to set NULL string on field '%s' on taglist. " - "Please file a bug.", g_quark_to_string (field->name)); - g_value_unset (&field->value); - return; - } else if (G_UNLIKELY (s != NULL && *s == '\0')) { - /* empty strings never make sense */ - g_warning ("Trying to set empty string on %s field '%s'. Please file a " - "bug.", IS_TAGLIST (structure) ? "taglist" : "structure", - g_quark_to_string (field->name)); - g_value_unset (&field->value); - return; + if (G_UNLIKELY (IS_TAGLIST (structure) && (s == NULL || *s == '\0'))) { + if (s == NULL) { + g_warning ("Trying to set NULL string on field '%s' on taglist. " + "Please file a bug.", g_quark_to_string (field->name)); + g_value_unset (&field->value); + return; + } else { + /* empty strings never make sense */ + g_warning ("Trying to set empty string on taglist field '%s'. " + "Please file a bug.", g_quark_to_string (field->name)); + g_value_unset (&field->value); + return; + } } else if (G_UNLIKELY (s != NULL && !g_utf8_validate (s, -1, NULL))) { g_warning ("Trying to set string on %s field '%s', but string is not " "valid UTF-8. Please file a bug.", @@ -1602,7 +1603,7 @@ priv_gst_structure_append_to_gstring (const GstStructure * structure, g_string_append_len (s, "=(", 2); g_string_append (s, gst_structure_to_abbr (type)); g_string_append_c (s, ')'); - g_string_append (s, GST_STR_NULL (t)); + g_string_append (s, t == NULL ? "NULL" : t); g_free (t); } @@ -1653,7 +1654,8 @@ gst_structure_to_string (const GstStructure * structure) * THIS FUNCTION MODIFIES THE STRING AND DETECTS INSIDE A NONTERMINATED STRING */ static gboolean -gst_structure_parse_string (gchar * s, gchar ** end, gchar ** next) +gst_structure_parse_string (gchar * s, gchar ** end, gchar ** next, + gboolean unescape) { gchar *w; @@ -1669,21 +1671,32 @@ gst_structure_parse_string (gchar * s, gchar ** end, gchar ** next) return ret; } - w = s; - s++; - while (*s != '"') { - if (*s == 0) - return FALSE; - - if (*s == '\\') { + if (unescape) { + w = s; + s++; + while (*s != '"') { + if (G_UNLIKELY (*s == 0)) + return FALSE; + if (G_UNLIKELY (*s == '\\')) + s++; + *w = *s; + w++; s++; } - - *w = *s; - w++; s++; + } else { + /* Find the closing quotes */ + s++; + while (*s != '"') { + if (G_UNLIKELY (*s == 0)) + return FALSE; + if (G_UNLIKELY (*s == '\\')) + s++; + s++; + } + s++; + w = s; } - s++; *end = w; *next = s; @@ -1931,11 +1944,7 @@ gst_structure_parse_value (gchar * str, ret = gst_structure_parse_array (s, &s, value, type); } else { value_s = s; - if (G_UNLIKELY (!gst_structure_parse_string (s, &value_end, &s))) - return FALSE; - c = *value_end; - *value_end = '\0'; if (G_UNLIKELY (type == G_TYPE_INVALID)) { GType try_types[] = { G_TYPE_INT, G_TYPE_DOUBLE, GST_TYPE_FRACTION, G_TYPE_BOOLEAN, @@ -1943,6 +1952,12 @@ gst_structure_parse_value (gchar * str, }; int i; + if (G_UNLIKELY (!gst_structure_parse_string (s, &value_end, &s, TRUE))) + return FALSE; + /* Set NULL terminator for deserialization */ + c = *value_end; + *value_end = '\0'; + for (i = 0; i < G_N_ELEMENTS (try_types); i++) { g_value_init (value, try_types[i]); ret = gst_value_deserialize (value, value_s); @@ -1953,6 +1968,13 @@ gst_structure_parse_value (gchar * str, } else { g_value_init (value, type); + if (G_UNLIKELY (!gst_structure_parse_string (s, &value_end, &s, + (type != G_TYPE_STRING)))) + return FALSE; + /* Set NULL terminator for deserialization */ + c = *value_end; + *value_end = '\0'; + ret = gst_value_deserialize (value, value_s); if (G_UNLIKELY (!ret)) g_value_unset (value); @@ -1999,7 +2021,7 @@ gst_structure_from_string (const gchar * string, gchar ** end) r++; name = r; - if (G_UNLIKELY (!gst_structure_parse_string (r, &w, &r))) { + if (G_UNLIKELY (!gst_structure_parse_string (r, &w, &r, TRUE))) { GST_WARNING ("Failed to parse structure string"); goto error; } diff --git a/gst/gstvalue.c b/gst/gstvalue.c index bbcfa1d4ac..358fa82030 100644 --- a/gst/gstvalue.c +++ b/gst/gstvalue.c @@ -1803,8 +1803,10 @@ gst_value_deserialize_float (GValue * dest, const gchar * s) static gint gst_value_compare_string (const GValue * value1, const GValue * value2) { - if (!value1->data[0].v_pointer || !value2->data[0].v_pointer) { - return GST_VALUE_UNORDERED; + if (G_UNLIKELY (!value1->data[0].v_pointer || !value2->data[0].v_pointer)) { + /* if only one is NULL, no match - otherwise both NULL == EQUAL */ + if (value1->data[0].v_pointer != value2->data[0].v_pointer) + return GST_VALUE_UNORDERED; } else { int x = strcmp (value1->data[0].v_pointer, value2->data[0].v_pointer); @@ -1812,8 +1814,9 @@ gst_value_compare_string (const GValue * value1, const GValue * value2) return GST_VALUE_LESS_THAN; if (x > 0) return GST_VALUE_GREATER_THAN; - return GST_VALUE_EQUAL; } + + return GST_VALUE_EQUAL; } static int @@ -1822,9 +1825,13 @@ gst_string_measure_wrapping (const gchar * s) int len; gboolean wrap = FALSE; - if (s == NULL) + if (G_UNLIKELY (s == NULL)) return -1; + /* Special case: the actual string NULL needs wrapping */ + if (G_UNLIKELY (strcmp (s, "NULL") == 0)) + return 4; + len = 0; while (*s) { if (GST_ASCII_IS_STRING (*s)) { @@ -1839,7 +1846,9 @@ gst_string_measure_wrapping (const gchar * s) s++; } - return wrap ? len : -1; + /* Wrap the string if we found something that needs + * wrapping, or the empty string (len == 0) */ + return (wrap || len == 0) ? len : -1; } static gchar * @@ -1866,6 +1875,7 @@ gst_string_wrap_inner (const gchar * s, int len) *e++ = '\"'; *e = 0; + g_assert (e - d <= len + 3); return d; } @@ -1875,7 +1885,7 @@ gst_string_wrap (const gchar * s) { int len = gst_string_measure_wrapping (s); - if (len < 0) + if (G_LIKELY (len < 0)) return g_strdup (s); return gst_string_wrap_inner (s, len); @@ -1888,7 +1898,7 @@ gst_string_take_and_wrap (gchar * s) gchar *out; int len = gst_string_measure_wrapping (s); - if (len < 0) + if (G_LIKELY (len < 0)) return s; out = gst_string_wrap_inner (s, len); @@ -1991,14 +2001,16 @@ gst_value_serialize_string (const GValue * value) static gboolean gst_value_deserialize_string (GValue * dest, const gchar * s) { - if (*s != '"') { + if (G_UNLIKELY (strcmp (s, "NULL") == 0)) { + g_value_set_string (dest, NULL); + return TRUE; + } else if (G_LIKELY (*s != '"')) { if (!g_utf8_validate (s, -1, NULL)) return FALSE; g_value_set_string (dest, s); return TRUE; } else { gchar *str = gst_string_unwrap (s); - if (G_UNLIKELY (!str)) return FALSE; g_value_take_string (dest, str); diff --git a/tests/check/gst/gststructure.c b/tests/check/gst/gststructure.c index bd126cf22b..ecc264e5e1 100644 --- a/tests/check/gst/gststructure.c +++ b/tests/check/gst/gststructure.c @@ -213,6 +213,40 @@ GST_START_TEST (test_complete_structure) GST_END_TEST; +GST_START_TEST (test_string_properties) +{ + GstCaps *caps1, *caps2; + GstStructure *st1, *st2; + gchar *str, *res1, *res2; + + /* test escaping/unescaping */ + st1 = gst_structure_new ("RandomStructure", "prop1", G_TYPE_STRING, "foo", + "prop2", G_TYPE_STRING, "", "prop3", G_TYPE_STRING, NULL, + "prop4", G_TYPE_STRING, "NULL", NULL); + str = gst_structure_to_string (st1); + st2 = gst_structure_from_string (str, NULL); + g_free (str); + + fail_unless (st2 != NULL); + + /* need to put stuctures into caps to compare */ + caps1 = gst_caps_new_empty (); + gst_caps_append_structure (caps1, st1); + caps2 = gst_caps_new_empty (); + gst_caps_append_structure (caps2, st2); + res1 = gst_caps_to_string (caps1); + res2 = gst_caps_to_string (caps2); + fail_unless (gst_caps_is_equal (caps1, caps2), + "Structures did not match:\n\tStructure 1: %s\n\tStructure 2: %s\n", + res1, res2); + gst_caps_unref (caps1); + gst_caps_unref (caps2); + g_free (res1); + g_free (res2); +} + +GST_END_TEST; + GST_START_TEST (test_structure_new) { GstStructure *s; @@ -421,18 +455,6 @@ GST_START_TEST (test_structure_nested_from_and_to_string) GST_END_TEST; -GST_START_TEST (test_empty_string_fields) -{ - GstStructure *s; - - s = gst_structure_empty_new ("con/struct"); - ASSERT_WARNING (gst_structure_set (s, "layout", G_TYPE_STRING, "", NULL)); - gst_structure_set (s, "debug-string", G_TYPE_STRING, NULL, NULL); - gst_structure_free (s); -} - -GST_END_TEST; - GST_START_TEST (test_vararg_getters) { GstStructure *s; @@ -546,13 +568,13 @@ gst_structure_suite (void) tcase_add_test (tc_chain, test_from_string); tcase_add_test (tc_chain, test_to_string); tcase_add_test (tc_chain, test_to_from_string); + tcase_add_test (tc_chain, test_string_properties); tcase_add_test (tc_chain, test_complete_structure); tcase_add_test (tc_chain, test_structure_new); tcase_add_test (tc_chain, test_fixate); tcase_add_test (tc_chain, test_fixate_frac_list); tcase_add_test (tc_chain, test_structure_nested); tcase_add_test (tc_chain, test_structure_nested_from_and_to_string); - tcase_add_test (tc_chain, test_empty_string_fields); tcase_add_test (tc_chain, test_vararg_getters); return s; } diff --git a/tests/check/gst/gstvalue.c b/tests/check/gst/gstvalue.c index 73cebe10bc..acdfa22ff0 100644 --- a/tests/check/gst/gstvalue.c +++ b/tests/check/gst/gstvalue.c @@ -428,7 +428,8 @@ GST_START_TEST (test_string) gchar *try[] = { "Dude", "Hi, I'm a string", - "tüüüt!" + "tüüüt!", + "\"\"" /* Empty string */ }; gchar *tmp; GValue v = { 0, }; @@ -465,7 +466,8 @@ GST_START_TEST (test_deserialize_string) { "", ""}, /* empty strings */ { - "\"\"", ""}, /* FAILURES */ + "\"\"", ""}, /* quoted empty string -> empty string */ + /* Expected FAILURES: */ { "\"", NULL}, /* missing second quote */ { @@ -538,6 +540,12 @@ GST_START_TEST (test_value_compare) fail_unless (gst_value_compare (&value1, &value2) == GST_VALUE_LESS_THAN); fail_unless (gst_value_compare (&value2, &value1) == GST_VALUE_GREATER_THAN); fail_unless (gst_value_compare (&value1, &value1) == GST_VALUE_EQUAL); + /* Test some NULL string comparisons */ + g_value_set_string (&value2, NULL); + fail_unless (gst_value_compare (&value1, &value2) == GST_VALUE_UNORDERED); + fail_unless (gst_value_compare (&value2, &value1) == GST_VALUE_UNORDERED); + fail_unless (gst_value_compare (&value2, &value2) == GST_VALUE_EQUAL); + g_value_unset (&value1); g_value_unset (&value2);