make names only settable when unparented, so we can guarantee uniqueness inside a parent together with the _add checks

Original commit message from CVS:
make names only settable when unparented, so we can guarantee
uniqueness inside a parent together with the _add checks

2

ChangeLog

@@ -1,3 +1,13 @@
+2004-12-13  Thomas Vander Stichele  <thomas at apestaart dot org>
+
+	* docs/design/part-conventions.txt:
+	* docs/design/part-gstobject.txt:
+	* gst/gstobject.c:
+	* gst/gstobject.h:
+	* check/gst/gstobject.c:
+	  make names only settable when unparented, so we can guarantee
+	  uniqueness inside a parent together with the _add checks
+
 2004-12-13  Thomas Vander Stichele  <thomas at apestaart dot org>
 
 	* check/Makefile.am:

check/gst/gstobject.c

@@ -187,21 +187,27 @@ G_STMT_START {				\
 #define MAIN_START_THREAD_FUNCTIONS(count, function, data)	\
 G_STMT_START {							\
   int i;							\
-  GThread *thread = NULL;					\
   for (i = 0; i < count; ++i) {					\
+    MAIN_START_THREAD_FUNCTION (i, function, data);		\
+  }								\
+} G_STMT_END;
+
+#define MAIN_START_THREAD_FUNCTION(i, function, data)		\
+G_STMT_START {							\
+    GThread *thread = NULL;					\
     g_message ("MAIN: creating thread %d\n", i);		\
     g_mutex_lock (mutex);					\
     thread = g_thread_create ((GThreadFunc) function, data,	\
 	TRUE, NULL);						\
-    thread_list = g_list_append (thread_list, thread);		\
-								\
     /* wait for thread to signal us that it's ready */		\
     g_message ("MAIN: waiting for thread %d\n", i);		\
     g_cond_wait (start_cond, mutex);				\
     g_mutex_unlock (mutex);					\
-  }								\
+								\
+    thread_list = g_list_append (thread_list, thread);		\
 } G_STMT_END;
 
+
 #define MAIN_SYNCHRONIZE()		\
 G_STMT_START {				\
   g_message ("MAIN: synchronizing\n");	\
@@ -338,7 +344,74 @@ START_TEST (test_fake_object_name_threaded_right)
   }
   MAIN_STOP_THREADS ();
 }
-END_TEST Suite * gst_object_suite (void)
+
+END_TEST
+/*
+ * main thread creates lots of objects
+ * child threads sets default names on objects
+ * then main thread checks uniqueness of object names
+ */
+    GList * object_list = NULL;
+gint num_objects = 1000;
+gint num_threads = 5;
+
+/* thread function for threaded default name change test */
+gpointer
+thread_name_object_default (int *i)
+{
+  int j;
+
+  THREAD_START ();
+
+  for (j = *i; j < num_objects; j += num_threads) {
+    GstObject *o = GST_OBJECT (g_list_nth_data (object_list, j));
+
+    gst_object_set_name (o, NULL);
+    /* a minimal sleep invokes a thread switch */
+    g_usleep (1);
+  }
+
+  /* thread is done, so let's return */
+  g_message ("THREAD %p: set name\n", g_thread_self ());
+
+  return NULL;
+}
+
+
+START_TEST (test_fake_object_name_threaded_unique)
+{
+  GstObject *object;
+  gint i;
+
+  g_message ("\nTEST: uniqueness of default names\n");
+
+  for (i = 0; i < num_objects; ++i) {
+    object = g_object_new (gst_fake_object_get_type (), NULL);
+    object_list = g_list_append (object_list, object);
+  }
+
+  MAIN_INIT ();
+
+  mark_point ();
+  for (i = 0; i < num_threads; ++i) {
+    MAIN_START_THREAD_FUNCTION (i, thread_name_object_default, &i);
+  }
+
+  mark_point ();
+  MAIN_SYNCHRONIZE ();
+  mark_point ();
+  MAIN_STOP_THREADS ();
+
+  /* sort GList based on object name */
+  /* FIXME: sort and test */
+
+  /* free stuff */
+  g_list_foreach (object_list, (GFunc) g_object_unref, NULL);
+}
+
+END_TEST
+/* test: try renaming a parented object, make sure it fails */
+    Suite * gst_object_suite (void)
 {
   Suite *s = suite_create ("GstObject");
   TCase *tc_chain = tcase_create ("general");
@@ -349,6 +422,7 @@ END_TEST Suite * gst_object_suite (void)
   tcase_add_test (tc_chain, test_fake_object_name);
   tcase_add_test (tc_chain, test_fake_object_name_threaded_wrong);
   tcase_add_test (tc_chain, test_fake_object_name_threaded_right);
+  tcase_add_test (tc_chain, test_fake_object_name_threaded_unique);
   //tcase_add_checked_fixture (tc_chain, setup, teardown);
   return s;
 }

docs/design/part-conventions.txt

@@ -16,7 +16,7 @@ Function names
 --------------
 
 Within the context of a given object, functions defined in that object's header and/or source file will have their 
-object-specific prefix stripped.  For instance, gst_element_add_pad() would be referred to as simply _get_pad().  Note 
+object-specific prefix stripped.  For instance, gst_element_add_pad() would be referred to as simply _add_pad().  Note 
 that the trailing parentheses should always be present, but sometimes may not be.  A prefixing underscore (_) will 
 always tell you it's a function, however, regardless of the presence or absence of the trailing parentheses.
 

docs/design/part-gstobject.txt

@@ -1,26 +1,35 @@
 GstObject
 =========
 
-The base class for the entire GStreamer hierarchy is the GstObject.  It is currently a bit of a hack caused by the
-fact that GStreamer is built on top of GtkObject.  GObject should help, if it's designed properly.  Currently the 
-capabilities provided by GstObject are quite underutilized, this will be fixed with a refactoring of the 
-object system and a code cleanup at some point in the future.  If nothing else, it serves as an easy check to see if a 
-given object belongs to GStreamer.
+The base class for the entire GStreamer hierarchy is the GstObject.
 
 Parentage
 ---------
 
-A pointer is available to store the current parent of the object.  This one of the two fundamental requires for a 
+A pointer is available to store the current parent of the object.
+This one of the two fundamental requires for a 
 hierarchical system such as GStreamer (for the other, read up on GstBin).  Three functions are provided: _set_parent(), 
 _get_parent(), and _unparent().  The third is required because there is an explicit check in _set_parent(): an object 
 must not already have a parent if you wish to set one.  You must unparent the object first.  This allows for new 
 additions later.
 
+- GstObject's that can be parented:
+  GstElement (inside a bin)
+  GstPad (inside an element)
+
 Refcounting
 -----------
+- GObject refcount is not threadsafe.
+  GStreamer sets it to a constant value on each _ref/_unref()
+  and uses an atomic int "refcount" instead for threadsafe refcounting
+  This implies you should always use gst_object_ref() and gst_object_unref() !
 
-A reference count is kept for the object.  When this is fully utilized, it will enable generic destruction of objects 
-by simply reducing the reference count to zero.  GObject should provide these capabilities, however.
+Naming
+------
+- names of objects cannot be changed when they are parented 
+- names of objects should be unique across parent
+- set_name() can fail because of this
+- as can gst_element_add_pad()/gst_bin_add_element()
 
 Locking
 -------

gst/gstobject.c

@@ -612,23 +612,32 @@ gst_object_set_name_default (GstObject * object)
  * This function makes a copy of the provided name, so the caller
  * retains ownership of the name it sent.
  *
- * MT safe.
+ * Returns: TRUE if the name could be set.
+ *
+ * MT safe.  This function grabs and releases the object's LOCK.
  */
-void
+gboolean
 gst_object_set_name (GstObject * object, const gchar * name)
 {
-  g_return_if_fail (GST_IS_OBJECT (object));
+  g_return_val_if_fail (GST_IS_OBJECT (object), FALSE);
 
   GST_LOCK (object);
-  g_free (object->name);
+
+  /* parented objects cannot be renamed */
+  if (object->parent != NULL) {
+    GST_UNLOCK (object);
+    return FALSE;
+  }
 
   if (name != NULL) {
+    g_free (object->name);
     object->name = g_strdup (name);
     GST_UNLOCK (object);
   } else {
     GST_UNLOCK (object);
     gst_object_set_name_default (object);
   }
+  return TRUE;
 }
 
 /**

gst/gstobject.h

@@ -133,7 +133,7 @@ struct _GstObjectClass {
 GType		gst_object_get_type		(void);
 
 /* name routines */
-void		gst_object_set_name		(GstObject *object, const gchar *name);
+gboolean	gst_object_set_name		(GstObject *object, const gchar *name);
 gchar* 		gst_object_get_name		(GstObject *object);
 
 /* parentage routines */

tests/check/gst/gstobject.c

@@ -187,21 +187,27 @@ G_STMT_START {				\
 #define MAIN_START_THREAD_FUNCTIONS(count, function, data)	\
 G_STMT_START {							\
   int i;							\
-  GThread *thread = NULL;					\
   for (i = 0; i < count; ++i) {					\
+    MAIN_START_THREAD_FUNCTION (i, function, data);		\
+  }								\
+} G_STMT_END;
+
+#define MAIN_START_THREAD_FUNCTION(i, function, data)		\
+G_STMT_START {							\
+    GThread *thread = NULL;					\
     g_message ("MAIN: creating thread %d\n", i);		\
     g_mutex_lock (mutex);					\
     thread = g_thread_create ((GThreadFunc) function, data,	\
 	TRUE, NULL);						\
-    thread_list = g_list_append (thread_list, thread);		\
-								\
     /* wait for thread to signal us that it's ready */		\
     g_message ("MAIN: waiting for thread %d\n", i);		\
     g_cond_wait (start_cond, mutex);				\
     g_mutex_unlock (mutex);					\
-  }								\
+								\
+    thread_list = g_list_append (thread_list, thread);		\
 } G_STMT_END;
 
+
 #define MAIN_SYNCHRONIZE()		\
 G_STMT_START {				\
   g_message ("MAIN: synchronizing\n");	\
@@ -338,7 +344,74 @@ START_TEST (test_fake_object_name_threaded_right)
   }
   MAIN_STOP_THREADS ();
 }
-END_TEST Suite * gst_object_suite (void)
+
+END_TEST
+/*
+ * main thread creates lots of objects
+ * child threads sets default names on objects
+ * then main thread checks uniqueness of object names
+ */
+    GList * object_list = NULL;
+gint num_objects = 1000;
+gint num_threads = 5;
+
+/* thread function for threaded default name change test */
+gpointer
+thread_name_object_default (int *i)
+{
+  int j;
+
+  THREAD_START ();
+
+  for (j = *i; j < num_objects; j += num_threads) {
+    GstObject *o = GST_OBJECT (g_list_nth_data (object_list, j));
+
+    gst_object_set_name (o, NULL);
+    /* a minimal sleep invokes a thread switch */
+    g_usleep (1);
+  }
+
+  /* thread is done, so let's return */
+  g_message ("THREAD %p: set name\n", g_thread_self ());
+
+  return NULL;
+}
+
+
+START_TEST (test_fake_object_name_threaded_unique)
+{
+  GstObject *object;
+  gint i;
+
+  g_message ("\nTEST: uniqueness of default names\n");
+
+  for (i = 0; i < num_objects; ++i) {
+    object = g_object_new (gst_fake_object_get_type (), NULL);
+    object_list = g_list_append (object_list, object);
+  }
+
+  MAIN_INIT ();
+
+  mark_point ();
+  for (i = 0; i < num_threads; ++i) {
+    MAIN_START_THREAD_FUNCTION (i, thread_name_object_default, &i);
+  }
+
+  mark_point ();
+  MAIN_SYNCHRONIZE ();
+  mark_point ();
+  MAIN_STOP_THREADS ();
+
+  /* sort GList based on object name */
+  /* FIXME: sort and test */
+
+  /* free stuff */
+  g_list_foreach (object_list, (GFunc) g_object_unref, NULL);
+}
+
+END_TEST
+/* test: try renaming a parented object, make sure it fails */
+    Suite * gst_object_suite (void)
 {
   Suite *s = suite_create ("GstObject");
   TCase *tc_chain = tcase_create ("general");
@@ -349,6 +422,7 @@ END_TEST Suite * gst_object_suite (void)
   tcase_add_test (tc_chain, test_fake_object_name);
   tcase_add_test (tc_chain, test_fake_object_name_threaded_wrong);
   tcase_add_test (tc_chain, test_fake_object_name_threaded_right);
+  tcase_add_test (tc_chain, test_fake_object_name_threaded_unique);
   //tcase_add_checked_fixture (tc_chain, setup, teardown);
   return s;
 }