Revert serialisation change and constrain structure-names after consensus on irc...

Original commit message from CVS:
* gst/gststructure.c:
* tests/check/gst/gststructure.c:
Revert serialisation change and constrain structure-names after
consensus on irc. Update api documentation to reflect the change.

diff --git a/ChangeLog b/ChangeLog
index 2449af2..92741da 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,13 @@
 2007-10-16  Stefan Kost  <ensonic@users.sf.net>
 
        * gst/gststructure.c:
+       * tests/check/gst/gststructure.c:
+         Revert serialisation change and constrain structure-names after
+         consensus on irc. Update api documentation to reflect the change.
+
+2007-10-16  Stefan Kost  <ensonic@users.sf.net>
+
+       * gst/gststructure.c:
          Improve serialization and fix tests.
 
        * tests/check/gst/gststructure.c:

diff --git a/gst/gststructure.c b/gst/gststructure.c
index 1b9d85c..a17fa61 100644 (file)
--- a/gst/gststructure.c
+++ b/gst/gststructure.c
@@ -27,7 +27,8 @@
  * A #GstStructure is a collection of key/value pairs. The keys are expressed
  * as GQuarks and the values can be of any GType.
  *
- * In addition to the key/value pairs, a #GstStructure also has a name.
+ * In addition to the key/value pairs, a #GstStructure also has a name. The name
+ * starts with a letter and can be folled by letters, numbers and any of "/-_.:".
  * 
  * #GstStructure is used by various GStreamer subsystems to store information
  * in a flexible and extensible way. A #GstStructure does not have a refcount
@@ -48,7 +49,7 @@
  * Fields can be removed with gst_structure_remove_field() or
  * gst_structure_remove_fields().
  *
- * Last reviewed on 2005-11-09 (0.9.4)
+ * Last reviewed on 2007-10-16 (0.10.15)
  */
 
 #ifdef HAVE_CONFIG_H
@@ -137,18 +138,46 @@ gst_structure_id_empty_new (GQuark quark)
   return gst_structure_id_empty_new_with_size (quark, 0);
 }
 
+static gboolean
+gst_structure_validate_name (const gchar * name)
+{
+  const gchar *s;
+
+  g_return_val_if_fail (name != NULL, FALSE);
+
+  if (!g_ascii_isalpha (*name)) {
+    GST_WARNING ("Invalid character '%c' at offset 0 in structure name: %s",
+        *name, name);
+    return FALSE;
+  }
+
+  /* FIXME: test name string more */
+  s = &name[1];
+  while (*s && (g_ascii_isalnum (*s) || strchr ("/-_.:", *s) != NULL))
+    s++;
+  if (*s != '\0') {
+    GST_WARNING ("Invalid character '%c' at offset %lu in structure name: %s",
+        *s, ((gulong) s - (gulong) name), name);
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
 /**
  * gst_structure_empty_new:
  * @name: name of new structure
  *
- * Creates a new, empty #GstStructure with the given name.
+ * Creates a new, empty #GstStructure with the given @name.
+ *
+ * See gst_structure_set_name() for constraints on the @name parameter.
  *
  * Returns: a new, empty #GstStructure
  */
 GstStructure *
 gst_structure_empty_new (const gchar * name)
 {
-  g_return_val_if_fail (name != NULL, NULL);
+  g_return_val_if_fail (gst_structure_validate_name (name), NULL);
 
   return gst_structure_id_empty_new_with_size (g_quark_from_string (name), 0);
 }
@@ -189,9 +218,11 @@ gst_structure_new (const gchar * name, const gchar * firstfield, ...)
  * @firstfield: name of first field to set
  * @varargs: variable argument list
  *
- * Creates a new #GstStructure with the given name.  Structure fields
+ * Creates a new #GstStructure with the given @name.  Structure fields
  * are set according to the varargs in a manner similar to
- * @gst_structure_new.
+ * gst_structure_new().
+ *
+ * See gst_structure_set_name() for constraints on the @name parameter.
  *
  * Returns: a new #GstStructure
  */
@@ -331,6 +362,9 @@ gst_structure_has_name (const GstStructure * structure, const gchar * name)
   g_return_val_if_fail (structure != NULL, FALSE);
   g_return_val_if_fail (name != NULL, FALSE);
 
+  /* getting the string is cheap and comparing short strings is too
+   * should be faster than getting the quark for name and comparing the quarks
+   */
   structure_name = g_quark_to_string (structure->name);
 
   return (structure_name && strcmp (structure_name, name) == 0);
@@ -357,15 +391,16 @@ gst_structure_get_name_id (const GstStructure * structure)
  * @structure: a #GstStructure
  * @name: the new name of the structure
  *
- * Sets the name of the structure to the given name.  The string
- * provided is copied before being used.
+ * Sets the name of the structure to the given @name.  The string
+ * provided is copied before being used. It must be not empty, start with a
+ * letter and can be folled by letters, numbers and any of "/-_.:".
  */
 void
 gst_structure_set_name (GstStructure * structure, const gchar * name)
 {
   g_return_if_fail (structure != NULL);
-  g_return_if_fail (name != NULL);
   g_return_if_fail (IS_MUTABLE (structure));
+  g_return_if_fail (gst_structure_validate_name (name));
 
   structure->name = g_quark_from_string (name);
 }
@@ -1484,8 +1519,6 @@ gst_structure_to_string (const GstStructure * structure)
   GstStructureField *field;
   GString *s;
   guint i;
-  const gchar *name;
-  gchar *name_esc;
 
   /* NOTE:  This function is potentially called by the debug system,
    * so any calls to gst_log() (and GST_DEBUG(), GST_LOG(), etc.)
@@ -1496,15 +1529,7 @@ gst_structure_to_string (const GstStructure * structure)
   g_return_val_if_fail (structure != NULL, NULL);
 
   s = g_string_new ("");
-  /* this string may need to be escaped */
-  name = g_quark_to_string (structure->name);
-  name_esc = g_strescape (name, NULL);
-  if ((strlen (name) < strlen (name_esc)) || strchr (name, ' ')) {
-    g_string_append_printf (s, "\"%s\"", name);
-  } else {
-    g_string_append_printf (s, "%s", name);
-  }
-  g_free (name_esc);
+  g_string_append_printf (s, "%s", g_quark_to_string (structure->name));
   for (i = 0; i < structure->fields->len; i++) {
     char *t;
     GType type;

diff --git a/tests/check/gst/gststructure.c b/tests/check/gst/gststructure.c
index fcf79b4..de57f96 100644 (file)
--- a/tests/check/gst/gststructure.c
+++ b/tests/check/gst/gststructure.c
@@ -122,21 +122,16 @@ GST_END_TEST;
 
 GST_START_TEST (test_to_string)
 {
-  GstStructure *st1, *st2;
-  gchar *str;
+  GstStructure *st1;
 
-  /* use structure name and string with spaces, to test escaping/unescaping */
-  st1 = gst_structure_new ("Foo Bar\nwith newline", "num", G_TYPE_INT, 9173,
-      "string", G_TYPE_STRING, "Something Like Face/Off", NULL);
-  str = gst_structure_to_string (st1);
-  st2 = gst_structure_from_string (str, NULL);
-  g_free (str);
+  ASSERT_CRITICAL (st1 = gst_structure_new ("Foo\nwith-newline", NULL));
+  fail_unless (st1 == NULL);
 
-  fail_unless (st2 != NULL);
-  fail_unless (!strcmp ("Foo Bar\nwith newline", gst_structure_get_name (st2)));
+  ASSERT_CRITICAL (st1 = gst_structure_new ("Foo with whitespace", NULL));
+  fail_unless (st1 == NULL);
 
-  gst_structure_free (st2);
-  gst_structure_free (st1);
+  ASSERT_CRITICAL (st1 = gst_structure_new ("1st", NULL));
+  fail_unless (st1 == NULL);
 }
 
 GST_END_TEST;
@@ -148,8 +143,8 @@ GST_START_TEST (test_to_from_string)
   GstStructure *st1, *st2;
   gchar *str, *res1, *res2;
 
-  /* use structure name and string with spaces, to test escaping/unescaping */
-  st1 = gst_structure_new ("Foo Bar", "num", G_TYPE_INT, 9173,
+  /* test escaping/unescaping */
+  st1 = gst_structure_new ("FooBar-123/0_1", "num", G_TYPE_INT, 9173,
       "string", G_TYPE_STRING, "Something Like Face/Off", NULL);
   str = gst_structure_to_string (st1);
   st2 = gst_structure_from_string (str, NULL);