ayatana-commits team mailing list archive

[noreply@xxxxxxxxxxxxx]

Merge authors:
  Ted Gould (ted)
Related merge proposals:
  https://code.launchpad.net/~ted/dbusmenu/proxy-text-fix/+merge/51848
  proposed by: Ted Gould (ted)
  review: Approve - Conor Curran (cjcurran)
------------------------------------------------------------
revno: 251 [merge]
committer: Ted Gould <ted@xxxxxxxx>
branch nick: trunk
timestamp: Wed 2011-03-02 09:35:04 -0600
message:
  Handle the case where the passed in property name could be from the hashtable.
modified:
  libdbusmenu-glib/client.c
  libdbusmenu-glib/menuitem.c


--
lp:dbusmenu
https://code.launchpad.net/~dbusmenu-team/dbusmenu/trunk

Your team ayatana-commits is subscribed to branch lp:dbusmenu.
To unsubscribe from this branch go to https://code.launchpad.net/~dbusmenu-team/dbusmenu/trunk/+edit-subscription

=== modified file 'libdbusmenu-glib/client.c'
--- libdbusmenu-glib/client.c	2011-03-02 11:07:07 +0000
+++ libdbusmenu-glib/client.c	2011-03-02 15:35:04 +0000
@@ -1168,7 +1168,7 @@
 			gchar * property;
 
 			while (g_variant_iter_next(&properties, "s", &property)) {
-				g_debug("Removing property '%s' on %d", property, id);
+				/* g_debug("Removing property '%s' on %d", property, id); */
 				dbusmenu_menuitem_property_remove(menuitem, property);
 			}
 		}

=== modified file 'libdbusmenu-glib/menuitem.c'
--- libdbusmenu-glib/menuitem.c	2011-03-01 11:15:19 +0000
+++ libdbusmenu-glib/menuitem.c	2011-03-02 03:12:03 +0000
@@ -1146,6 +1146,7 @@
 {
 	g_return_val_if_fail(DBUSMENU_IS_MENUITEM(mi), FALSE);
 	g_return_val_if_fail(property != NULL, FALSE);
+	g_return_val_if_fail(g_utf8_validate(property, -1, NULL), FALSE);
 
 	DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
 	GVariant * default_value = NULL;
@@ -1181,6 +1182,7 @@
 
 
 	gboolean replaced = FALSE;
+	gboolean remove = FALSE;
 	gpointer currentval = g_hash_table_lookup(priv->properties, property);
 
 	if (value != NULL) {
@@ -1195,10 +1197,21 @@
 		gchar * lprop = g_strdup(property);
 		g_variant_ref_sink(value);
 
-		g_hash_table_replace(priv->properties, lprop, value);
+		/* Really important that this is _insert as that means the value
+		   that we just created in the _strdup is free'd and not the one
+		   currently in the hashtable.  That could be the same as the one
+		   being passed in and then the signal emit would be done with a
+		   bad value */
+		g_hash_table_insert(priv->properties, lprop, value);
 	} else {
 		if (currentval != NULL) {
-			g_hash_table_remove(priv->properties, property);
+		/* So the question you should be asking if you're paying attention
+		   is "Why not just do the remove here?"  It's a good question with
+		   an interesting answer.  Bascially it's the same reason as above,
+		   in a couple cases the passed in properties is the value in the hash
+		   table so we can avoid strdup'ing it by removing it (and thus free'ing
+		   it) after the signal emition */
+			remove = TRUE;
 			replaced = TRUE;
 		}
 	}
@@ -1219,6 +1232,10 @@
 		g_signal_emit(G_OBJECT(mi), signals[PROPERTY_CHANGED], 0, property, signalval, TRUE);
 	}
 
+	if (remove) {
+		g_hash_table_remove(priv->properties, property);
+	}
+
 	return TRUE;
 }