ayatana-commits team mailing list archive

[Ted Gould <ted@xxxxxxxx>]

Ted Gould has proposed merging lp:~ted/dbusmenu/extra-references-track into lp:dbusmenu.

Requested reviews:
  DBus Menu Team (dbusmenu-team)


Extra refs.  There were some extra refs, and cleaning up those exposed
situations where they were holding the fort down.  Basically the async
property updates were assuming memory leaks.  Nice.
-- 
https://code.launchpad.net/~ted/dbusmenu/extra-references-track/+merge/23571
Your team ayatana-commits is subscribed to branch lp:dbusmenu.

=== modified file 'libdbusmenu-glib/client.c'
--- libdbusmenu-glib/client.c	2010-04-15 04:41:32 +0000
+++ libdbusmenu-glib/client.c	2010-04-16 18:20:34 +0000
@@ -367,6 +367,7 @@
 
 	gchar * properties[1] = {NULL}; /* This gets them all */
 	g_debug("Getting properties");
+	g_object_ref(menuitem);
 	org_ayatana_dbusmenu_get_properties_async(proxy, id, (const gchar **)properties, menuitem_get_properties_cb, menuitem);
 	return;
 }
@@ -574,10 +575,12 @@
 	g_return_if_fail(DBUSMENU_IS_MENUITEM(data));
 	if (error != NULL) {
 		g_warning("Error getting properties on a menuitem: %s", error->message);
+		g_object_unref(data);
 		return;
 	}
 	g_hash_table_foreach(properties, get_properties_helper, data);
 	g_hash_table_destroy(properties);
+	g_object_unref(data);
 	return;
 }
 
@@ -606,6 +609,8 @@
 
 	if (!have_error) {
 		menuitem_get_properties_cb(proxy, properties, error, data);
+	} else {
+		g_object_unref(data);
 	}
 
 	return;
@@ -618,6 +623,7 @@
 {
 	if (error != NULL) {
 		g_warning("Error getting properties on a new menuitem: %s", error->message);
+		g_object_unref(data);
 		return;
 	}
 	g_return_if_fail(data != NULL);
@@ -625,6 +631,7 @@
 	newItemPropData * propdata = (newItemPropData *)data;
 	DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(propdata->client);
 
+	g_object_ref(propdata->item);
 	menuitem_get_properties_cb (proxy, properties, error, propdata->item);
 
 	gboolean handled = FALSE;
@@ -652,6 +659,7 @@
 		g_signal_emit(G_OBJECT(propdata->client), signals[NEW_MENUITEM], 0, propdata->item, TRUE);
 	}
 
+	g_object_unref(propdata->item);
 	g_free(propdata);
 
 	return;
@@ -749,8 +757,6 @@
 			if (parent != NULL) {
 				dbusmenu_menuitem_child_delete(parent, item);
 			}
-			/* XXX: Should this be an unref?  Who's reffing this that it exists without a parent? */
-			g_object_unref(G_OBJECT(item));
 			item = NULL;
 		}
 
@@ -769,6 +775,7 @@
 			propdata->parent  = parent;
 
 			gchar * properties[1] = {NULL}; /* This gets them all */
+			g_object_ref(item);
 			org_ayatana_dbusmenu_get_properties_async(proxy, id, (const gchar **)properties, menuitem_get_properties_new_cb, propdata);
 		} else {
 			g_warning("Unable to allocate memory to get properties for menuitem.  This menuitem will never be realized.");
@@ -777,6 +784,7 @@
 		/* Refresh the properties */
 		/* XXX: We shouldn't need to get the properties everytime we reuse an entry */
 		gchar * properties[1] = {NULL}; /* This gets them all */
+		g_object_ref(item);
 		org_ayatana_dbusmenu_get_properties_async(proxy, id, (const gchar **)properties, menuitem_get_properties_replace_cb, item);
 	}
 
@@ -810,6 +818,7 @@
 				dbusmenu_menuitem_child_delete(item, childmi);
 			}
 			dbusmenu_menuitem_child_add_position(item, newchildmi, position);
+			g_object_unref(newchildmi);
 		} else {
 			dbusmenu_menuitem_child_reorder(item, childmi, position);
 		}
@@ -847,9 +856,6 @@
 	xmlNodePtr root = xmlDocGetRootElement(xmldoc);
 
 	DbusmenuMenuitem * oldroot = priv->root;
-	if (oldroot != NULL) {
-		g_object_ref(oldroot);
-	}
 
 	priv->root = parse_layout_xml(client, root, priv->root, NULL, priv->menuproxy);
 	xmlFreeDoc(xmldoc);
@@ -867,17 +873,14 @@
 		   clean up that old root */
 		if (oldroot != NULL) {
 			dbusmenu_menuitem_set_root(oldroot, FALSE);
+			g_object_unref(oldroot);
+			oldroot = NULL;
 		}
 
 		/* If the root changed we can signal that */
 		g_signal_emit(G_OBJECT(client), signals[ROOT_CHANGED], 0, priv->root, TRUE);
 	}
 
-	/* We need to unref it in this function no matter */
-	if (oldroot != NULL) {
-		g_object_unref(oldroot);
-	}
-
 	return 1;
 }
 

=== modified file 'libdbusmenu-glib/menuitem.c'
--- libdbusmenu-glib/menuitem.c	2010-04-15 04:47:01 +0000
+++ libdbusmenu-glib/menuitem.c	2010-04-16 18:20:34 +0000
@@ -491,8 +491,8 @@
 	#ifdef MASSIVEDEBUGGING
 	g_debug("Menuitem %d (%s) signalling child removed %d (%s)", ID(user_data), LABEL(user_data), ID(data), LABEL(data));
 	#endif
+	g_signal_emit(G_OBJECT(user_data), signals[CHILD_REMOVED], 0, DBUSMENU_MENUITEM(data), TRUE);
 	g_object_unref(G_OBJECT(data));
-	g_signal_emit(G_OBJECT(user_data), signals[CHILD_REMOVED], 0, DBUSMENU_MENUITEM(data), TRUE);
 	return;
 }
 
@@ -681,8 +681,8 @@
 	#ifdef MASSIVEDEBUGGING
 	g_debug("Menuitem %d (%s) signalling child removed %d (%s)", ID(mi), LABEL(mi), ID(child), LABEL(child));
 	#endif
+	g_signal_emit(G_OBJECT(mi), signals[CHILD_REMOVED], 0, child, TRUE);
 	g_object_unref(G_OBJECT(child));
-	g_signal_emit(G_OBJECT(mi), signals[CHILD_REMOVED], 0, child, TRUE);
 	return TRUE;
 }