Do name collision checking more properly.

4 files changed, 122 insertions, 38 deletions

diff --git a/liblogdiag/ld-library.c b/liblogdiag/ld-library.c
index 8f07720..4d5d3e6 100644
--- a/liblogdiag/ld-library.c
+++ b/liblogdiag/ld-library.c
@@ -50,6 +50,8 @@ static gboolean foreach_dir (const gchar *path,
 	gpointer userdata, GError **error);
 static gboolean ld_library_load_cb
 	(const gchar *base, const gchar *filename, gpointer userdata);
+static void on_category_notify_name (LdSymbolCategory *category,
+	GParamSpec *pspec, gpointer user_data);
 
 
 G_DEFINE_TYPE (LdLibrary, ld_library, G_TYPE_OBJECT);
@@ -87,6 +89,14 @@ ld_library_init (LdLibrary *self)
 }
 
 static void
+uninstall_category_cb (LdSymbolCategory *category, LdLibrary *self)
+{
+	g_signal_handlers_disconnect_by_func (category,
+		on_category_notify_name, self);
+	g_object_unref (category);
+}
+
+static void
 ld_library_finalize (GObject *gobject)
 {
 	LdLibrary *self;
@@ -95,7 +105,7 @@ ld_library_finalize (GObject *gobject)
 
 	g_object_unref (self->priv->lua);
 
-	g_slist_foreach (self->priv->children, (GFunc) g_object_unref, NULL);
+	g_slist_foreach (self->priv->children, (GFunc) uninstall_category_cb, self);
 	g_slist_free (self->priv->children);
 
 	/* Chain up to the parent class. */
@@ -238,29 +248,12 @@ load_category_cb (const gchar *base, const gchar *filename, gpointer userdata)
 static void
 load_category_symbol_cb (LdSymbol *symbol, gpointer user_data)
 {
-	const gchar *name;
 	LdSymbolCategory *cat;
-	const GSList *symbols, *iter;
 
 	g_return_if_fail (LD_IS_SYMBOL (symbol));
 	g_return_if_fail (LD_IS_SYMBOL_CATEGORY (user_data));
 
 	cat = LD_SYMBOL_CATEGORY (user_data);
-	name = ld_symbol_get_name (symbol);
-
-	/* Check for name collisions with other symbols. */
-	/* XXX: This check should probably be in _insert_symbol() and _category().
-	 *      And the warning should show the full path. */
-	symbols = ld_symbol_category_get_symbols (cat);
-	for (iter = symbols; iter; iter = iter->next)
-	{
-		if (!strcmp (name, ld_symbol_get_name (LD_SYMBOL (iter->data))))
-		{
-			g_warning ("attempted to insert multiple `%s' symbols into"
-				" category `%s'", name, ld_symbol_category_get_name (cat));
-			return;
-		}
-	}
 	ld_symbol_category_insert_symbol (cat, symbol, -1);
 }
 
@@ -473,7 +466,7 @@ ld_library_clear (LdLibrary *self)
 {
 	g_return_if_fail (LD_IS_LIBRARY (self));
 
-	g_slist_foreach (self->priv->children, (GFunc) g_object_unref, NULL);
+	g_slist_foreach (self->priv->children, (GFunc) uninstall_category_cb, self);
 	g_slist_free (self->priv->children);
 	self->priv->children = NULL;
 
@@ -481,6 +474,14 @@ ld_library_clear (LdLibrary *self)
 		LD_LIBRARY_GET_CLASS (self)->changed_signal, 0);
 }
 
+static void
+on_category_notify_name (LdSymbolCategory *category,
+	GParamSpec *pspec, gpointer user_data)
+{
+	/* XXX: We could disown the category if a name collision has occured. */
+	g_warning ("name of a library category has changed");
+}
+
 /**
  * ld_library_insert_category:
  * @self: an #LdLibrary object.
@@ -489,16 +490,36 @@ ld_library_clear (LdLibrary *self)
  *       Negative values will append to the end of list.
  *
  * Insert a child category into the library.
+ *
+ * Return value: %TRUE if successful (no name collisions).
  */
-void
+gboolean
 ld_library_insert_category (LdLibrary *self,
 	LdSymbolCategory *category, gint pos)
 {
-	g_return_if_fail (LD_IS_LIBRARY (self));
-	g_return_if_fail (LD_IS_SYMBOL_CATEGORY (category));
+	const gchar *name;
+	const GSList *iter;
 
-	g_object_ref (category);
+	g_return_val_if_fail (LD_IS_LIBRARY (self), FALSE);
+	g_return_val_if_fail (LD_IS_SYMBOL_CATEGORY (category), FALSE);
+
+	/* Check for name collisions. */
+	name = ld_symbol_category_get_name (category);
+	for (iter = self->priv->children; iter; iter = iter->next)
+	{
+		if (!strcmp (name, ld_symbol_category_get_name (iter->data)))
+		{
+			g_warning ("attempted to insert multiple `%s' categories into"
+				" library", name);
+			return FALSE;
+		}
+	}
+
+	g_signal_connect (category, "notify::name",
+		G_CALLBACK (on_category_notify_name), self);
 	self->priv->children = g_slist_insert (self->priv->children, category, pos);
+	g_object_ref (category);
+	return TRUE;
 }
 
 /**
@@ -516,8 +537,10 @@ ld_library_remove_category (LdLibrary *self, LdSymbolCategory *category)
 
 	if (g_slist_find (self->priv->children, category))
 	{
-		g_object_unref (category);
+		g_signal_handlers_disconnect_by_func (category,
+			on_category_notify_name, self);
 		self->priv->children = g_slist_remove (self->priv->children, category);
+		g_object_unref (category);
 	}
 }
 
diff --git a/liblogdiag/ld-library.h b/liblogdiag/ld-library.h
index dd05efa..7521492 100644
--- a/liblogdiag/ld-library.h
+++ b/liblogdiag/ld-library.h
@@ -62,7 +62,8 @@ gboolean ld_library_load (LdLibrary *self, const gchar *directory);
 LdSymbol *ld_library_find_symbol (LdLibrary *self, const gchar *identifier);
 void ld_library_clear (LdLibrary *self);
 
-void ld_library_insert_category (LdLibrary *self,
+/* FIXME: This duplicates a part of LdSymbolCategory. */
+gboolean ld_library_insert_category (LdLibrary *self,
 	LdSymbolCategory *category, gint pos);
 void ld_library_remove_category (LdLibrary *self,
 	LdSymbolCategory *category);
diff --git a/liblogdiag/ld-symbol-category.c b/liblogdiag/ld-symbol-category.c
index 4c35c4c..52763e2 100644
--- a/liblogdiag/ld-symbol-category.c
+++ b/liblogdiag/ld-symbol-category.c
@@ -50,6 +50,9 @@ static void ld_symbol_category_set_property (GObject *object, guint property_id,
 	const GValue *value, GParamSpec *pspec);
 static void ld_symbol_category_finalize (GObject *gobject);
 
+static void on_category_notify_name (LdSymbolCategory *category,
+	GParamSpec *pspec, gpointer user_data);
+
 
 G_DEFINE_TYPE (LdSymbolCategory, ld_symbol_category, G_TYPE_OBJECT);
 
@@ -151,6 +154,14 @@ ld_symbol_category_set_property (GObject *object, guint property_id,
 }
 
 static void
+uninstall_category_cb (LdSymbolCategory *category, LdSymbolCategory *self)
+{
+	g_signal_handlers_disconnect_by_func (category,
+		on_category_notify_name, self);
+	g_object_unref (category);
+}
+
+static void
 ld_symbol_category_finalize (GObject *gobject)
 {
 	LdSymbolCategory *self;
@@ -167,7 +178,8 @@ ld_symbol_category_finalize (GObject *gobject)
 	g_slist_foreach (self->priv->symbols, (GFunc) g_object_unref, NULL);
 	g_slist_free (self->priv->symbols);
 
-	g_slist_foreach (self->priv->subcategories, (GFunc) g_object_unref, NULL);
+	g_slist_foreach (self->priv->subcategories,
+		(GFunc) uninstall_category_cb, self);
 	g_slist_free (self->priv->subcategories);
 
 	/* Chain up to the parent class. */
@@ -298,17 +310,35 @@ ld_symbol_category_get_image_path (LdSymbolCategory *self)
  * @pos: the position at which the symbol will be inserted.
  *       Negative values will append to the end of list.
  *
- * Insert a symbol into the category.  Doesn't check for duplicates.
+ * Insert a symbol into the category.
+ *
+ * Return value: %TRUE if successful (no name collisions).
  */
-void
+gboolean
 ld_symbol_category_insert_symbol (LdSymbolCategory *self,
 	LdSymbol *symbol, gint pos)
 {
-	g_return_if_fail (LD_IS_SYMBOL_CATEGORY (self));
-	g_return_if_fail (LD_IS_SYMBOL (symbol));
+	const gchar *name;
+	const GSList *iter;
+
+	g_return_val_if_fail (LD_IS_SYMBOL_CATEGORY (self), FALSE);
+	g_return_val_if_fail (LD_IS_SYMBOL (symbol), FALSE);
+
+	/* Check for name collisions. */
+	name = ld_symbol_get_name (symbol);
+	for (iter = self->priv->symbols; iter; iter = iter->next)
+	{
+		if (!strcmp (name, ld_symbol_get_name (iter->data)))
+		{
+			g_warning ("attempted to insert multiple `%s' symbols into"
+				" category `%s'", name, ld_symbol_category_get_name (self));
+			return FALSE;
+		}
+	}
 
-	g_object_ref (symbol);
 	self->priv->symbols = g_slist_insert (self->priv->symbols, symbol, pos);
+	g_object_ref (symbol);
+	return TRUE;
 }
 
 /**
@@ -346,6 +376,14 @@ ld_symbol_category_get_symbols (LdSymbolCategory *self)
 }
 
 
+static void
+on_category_notify_name (LdSymbolCategory *category,
+	GParamSpec *pspec, gpointer user_data)
+{
+	/* XXX: We could disown the category if a name collision has occured. */
+	g_warning ("name of a library subcategory has changed");
+}
+
 /**
  * ld_symbol_category_insert_subcategory:
  * @self: an #LdSymbolCategory object.
@@ -353,18 +391,38 @@ ld_symbol_category_get_symbols (LdSymbolCategory *self)
  * @pos: the position at which the category will be inserted.
  *       Negative values will append to the end of list.
  *
- * Insert a subcategory into the category.  Doesn't check for duplicates.
+ * Insert a subcategory into the category.
+ *
+ * Return value: %TRUE if successful (no name collisions).
  */
-void
+gboolean
 ld_symbol_category_insert_subcategory (LdSymbolCategory *self,
 	LdSymbolCategory *category, gint pos)
 {
-	g_return_if_fail (LD_IS_SYMBOL_CATEGORY (self));
-	g_return_if_fail (LD_IS_SYMBOL_CATEGORY (category));
+	const gchar *name;
+	const GSList *iter;
 
-	g_object_ref (category);
+	g_return_val_if_fail (LD_IS_SYMBOL_CATEGORY (self), FALSE);
+	g_return_val_if_fail (LD_IS_SYMBOL_CATEGORY (category), FALSE);
+
+	/* Check for name collisions. */
+	name = ld_symbol_category_get_name (category);
+	for (iter = self->priv->subcategories; iter; iter = iter->next)
+	{
+		if (!strcmp (name, ld_symbol_category_get_name (iter->data)))
+		{
+			g_warning ("attempted to insert multiple `%s' subcategories into"
+				" category `%s'", name, ld_symbol_category_get_name (self));
+			return FALSE;
+		}
+	}
+
+	g_signal_connect (category, "notify::name",
+		G_CALLBACK (on_category_notify_name), self);
 	self->priv->subcategories
 		= g_slist_insert (self->priv->subcategories, category, pos);
+	g_object_ref (category);
+	return TRUE;
 }
 
 /**
@@ -383,6 +441,8 @@ ld_symbol_category_remove_subcategory (LdSymbolCategory *self,
 
 	if (g_slist_find (self->priv->subcategories, category))
 	{
+		g_signal_handlers_disconnect_by_func (category,
+			on_category_notify_name, self);
 		self->priv->subcategories
 			= g_slist_remove (self->priv->subcategories, category);
 		g_object_unref (category);
diff --git a/liblogdiag/ld-symbol-category.h b/liblogdiag/ld-symbol-category.h
index 6913a86..1ecb436 100644
--- a/liblogdiag/ld-symbol-category.h
+++ b/liblogdiag/ld-symbol-category.h
@@ -66,13 +66,13 @@ void ld_symbol_category_set_image_path (LdSymbolCategory *self,
 	const gchar *image_path);
 const gchar *ld_symbol_category_get_image_path (LdSymbolCategory *self);
 
-void ld_symbol_category_insert_symbol (LdSymbolCategory *self,
+gboolean ld_symbol_category_insert_symbol (LdSymbolCategory *self,
 	LdSymbol *symbol, gint pos);
 void ld_symbol_category_remove_symbol (LdSymbolCategory *self,
 	LdSymbol *symbol);
 const GSList *ld_symbol_category_get_symbols (LdSymbolCategory *self);
 
-void ld_symbol_category_insert_subcategory (LdSymbolCategory *self,
+gboolean ld_symbol_category_insert_subcategory (LdSymbolCategory *self,
 	LdSymbolCategory *category, gint pos);
 void ld_symbol_category_remove_subcategory (LdSymbolCategory *self,
 	LdSymbolCategory *category);