ayatana-commits team mailing list archive
-
ayatana-commits team
-
Mailing list archive
-
Message #03190
[Merge] lp:~chrisccoulson/dbusmenu/more-memory-fixes into lp:dbusmenu
Chris Coulson has proposed merging lp:~chrisccoulson/dbusmenu/more-memory-fixes into lp:dbusmenu.
Requested reviews:
Ted Gould (ted)
Related bugs:
#721915 gnome-terminal crashed with SIGSEGV in g_type_check_instance_cast()
https://bugs.launchpad.net/bugs/721915
#723839 valgrind: invalid read errors in widget_notify_cb()
https://bugs.launchpad.net/bugs/723839
For more details, see:
https://code.launchpad.net/~chrisccoulson/dbusmenu/more-memory-fixes/+merge/51152
More memory error fixes :)
--
https://code.launchpad.net/~chrisccoulson/dbusmenu/more-memory-fixes/+merge/51152
Your team ayatana-commits is subscribed to branch lp:dbusmenu.
=== modified file 'libdbusmenu-glib/menuitem.c'
--- libdbusmenu-glib/menuitem.c 2011-02-24 14:34:48 +0000
+++ libdbusmenu-glib/menuitem.c 2011-02-24 15:48:24 +0000
@@ -62,6 +62,7 @@
gboolean realized;
DbusmenuDefaults * defaults;
gboolean exposed;
+ DbusmenuMenuitem * parent;
};
/* Signals */
@@ -339,6 +340,11 @@
priv->defaults = NULL;
}
+ if (priv->parent) {
+ g_object_remove_weak_pointer(G_OBJECT(priv->parent), (gpointer *)&priv->parent);
+ priv->parent = NULL;
+ }
+
G_OBJECT_CLASS (dbusmenu_menuitem_parent_class)->dispose (object);
return;
}
@@ -565,11 +571,12 @@
/* For all the taken children we need to signal
that they were removed */
static void
-take_children_signal (gpointer data, gpointer user_data)
+take_children_helper (gpointer data, gpointer user_data)
{
#ifdef MASSIVEDEBUGGING
g_debug("Menuitem %d (%s) signalling child removed %d (%s)", ID(user_data), LABEL(user_data), ID(data), LABEL(data));
#endif
+ dbusmenu_menuitem_unparent(DBUSMENU_MENUITEM(data));
g_signal_emit(G_OBJECT(user_data), signals[CHILD_REMOVED], 0, DBUSMENU_MENUITEM(data), TRUE);
return;
}
@@ -595,7 +602,7 @@
DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
GList * children = priv->children;
priv->children = NULL;
- g_list_foreach(children, take_children_signal, mi);
+ g_list_foreach(children, take_children_helper, mi);
dbusmenu_menuitem_property_remove(mi, DBUSMENU_MENUITEM_PROP_CHILD_DISPLAY);
@@ -704,6 +711,10 @@
DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
g_return_val_if_fail(g_list_find(priv->children, child) == NULL, FALSE);
+ if (!dbusmenu_menuitem_set_parent(child, mi)) {
+ return FALSE;
+ }
+
if (priv->children == NULL && !dbusmenu_menuitem_property_exist(mi, DBUSMENU_MENUITEM_PROP_CHILD_DISPLAY)) {
dbusmenu_menuitem_property_set(mi, DBUSMENU_MENUITEM_PROP_CHILD_DISPLAY, DBUSMENU_MENUITEM_CHILD_DISPLAY_SUBMENU);
}
@@ -736,6 +747,10 @@
DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
g_return_val_if_fail(g_list_find(priv->children, child) == NULL, FALSE);
+ if (!dbusmenu_menuitem_set_parent(child, mi)) {
+ return FALSE;
+ }
+
if (priv->children == NULL && !dbusmenu_menuitem_property_exist(mi, DBUSMENU_MENUITEM_PROP_CHILD_DISPLAY)) {
dbusmenu_menuitem_property_set(mi, DBUSMENU_MENUITEM_PROP_CHILD_DISPLAY, DBUSMENU_MENUITEM_CHILD_DISPLAY_SUBMENU);
}
@@ -768,6 +783,7 @@
DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
priv->children = g_list_remove(priv->children, child);
+ dbusmenu_menuitem_unparent(child);
#ifdef MASSIVEDEBUGGING
g_debug("Menuitem %d (%s) signalling child removed %d (%s)", ID(mi), LABEL(mi), ID(child), LABEL(child));
#endif
@@ -802,6 +818,10 @@
DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
g_return_val_if_fail(g_list_find(priv->children, child) == NULL, FALSE);
+ if (!dbusmenu_menuitem_set_parent(child, mi)) {
+ return FALSE;
+ }
+
if (priv->children == NULL && !dbusmenu_menuitem_property_exist(mi, DBUSMENU_MENUITEM_PROP_CHILD_DISPLAY)) {
dbusmenu_menuitem_property_set(mi, DBUSMENU_MENUITEM_PROP_CHILD_DISPLAY, DBUSMENU_MENUITEM_CHILD_DISPLAY_SUBMENU);
}
@@ -937,6 +957,84 @@
}
/**
+ * dbusmenu_menuitem_set_parent:
+ * @mi: The #DbusmenuMenuitem for which to set the parent
+ * @parent: The new parent #DbusmenuMenuitem
+ *
+ * Sets the parent of @mi to @parent. If @mi already
+ * has a parent, then this call will fail. The parent will
+ * be set automatically when using the usual methods to add a
+ * child menuitem, so this function should not normally be
+ * called directly
+ *
+ * Return value: Whether the parent was set successfully
+ */
+gboolean
+dbusmenu_menuitem_set_parent (DbusmenuMenuitem * mi, DbusmenuMenuitem * parent)
+{
+ g_return_val_if_fail(DBUSMENU_IS_MENUITEM(mi), FALSE);
+ g_return_val_if_fail(DBUSMENU_IS_MENUITEM(mi), FALSE);
+
+ DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
+
+ if (priv->parent != NULL) {
+ g_warning ("Menu item already has a parent");
+ return FALSE;
+ }
+
+ priv->parent = parent;
+ g_object_add_weak_pointer(G_OBJECT(priv->parent), (gpointer *)&priv->parent);
+
+ return TRUE;
+}
+
+/**
+ * dbusmenu_menuitem_unparent:
+ * @mi: The #DbusmenuMenuitem to unparent
+ *
+ * Unparents the menu item @mi. If @mi doesn't have a
+ * parent, then this call will fail. The menuitem will
+ * be unparented automatically when using the usual methods
+ * to delete a child menuitem, so this function should not
+ * normally be called directly
+ *
+ * Return value: Whether the menu item was unparented successfully
+ */
+gboolean
+dbusmenu_menuitem_unparent (DbusmenuMenuitem * mi)
+{
+ g_return_val_if_fail(DBUSMENU_IS_MENUITEM(mi), FALSE);
+ DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
+
+ if (priv->parent == NULL) {
+ g_warning("Menu item doesn't have a parent");
+ return FALSE;
+ }
+
+ g_object_remove_weak_pointer(G_OBJECT(priv->parent), (gpointer *)&priv->parent);
+ priv->parent = NULL;
+
+ return TRUE;
+}
+
+/**
+ * dbusmenu_menuitem_get_parent:
+ * @mi: The #DbusmenuMenuitem for which to inspect the parent
+ *
+ * This function looks up the parent of @mi
+ *
+ * Return value: The parent of this menu item
+ */
+DbusmenuMenuitem *
+dbusmenu_menuitem_get_parent (DbusmenuMenuitem * mi)
+{
+ g_return_val_if_fail(DBUSMENU_IS_MENUITEM(mi), NULL);
+ DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi);
+
+ return priv->parent;
+}
+
+/**
* dbusmenu_menuitem_property_set:
* @mi: The #DbusmenuMenuitem to set the property on.
* @property: Name of the property to set.
=== modified file 'libdbusmenu-glib/menuitem.h'
--- libdbusmenu-glib/menuitem.h 2011-02-21 20:14:56 +0000
+++ libdbusmenu-glib/menuitem.h 2011-02-24 15:48:24 +0000
@@ -379,6 +379,10 @@
DbusmenuMenuitem * dbusmenu_menuitem_child_find (DbusmenuMenuitem * mi, gint id);
DbusmenuMenuitem * dbusmenu_menuitem_find_id (DbusmenuMenuitem * mi, gint id);
+gboolean dbusmenu_menuitem_set_parent (DbusmenuMenuitem * mi, DbusmenuMenuitem * parent);
+gboolean dbusmenu_menuitem_unparent (DbusmenuMenuitem *mi);
+DbusmenuMenuitem * dbusmenu_menuitem_get_parent (DbusmenuMenuitem * mi);
+
gboolean dbusmenu_menuitem_property_set (DbusmenuMenuitem * mi, const gchar * property, const gchar * value);
gboolean dbusmenu_menuitem_property_set_variant (DbusmenuMenuitem * mi, const gchar * property, GVariant * value);
gboolean dbusmenu_menuitem_property_set_bool (DbusmenuMenuitem * mi, const gchar * property, const gboolean value);
=== modified file 'libdbusmenu-gtk/parser.c'
--- libdbusmenu-gtk/parser.c 2011-02-23 14:34:36 +0000
+++ libdbusmenu-gtk/parser.c 2011-02-24 15:48:24 +0000
@@ -161,6 +161,15 @@
}
}
+static void
+widget_freed (gpointer data, GObject * obj)
+{
+ g_signal_handlers_disconnect_by_func(gtk_icon_theme_get_default(), G_CALLBACK(theme_changed_cb), obj);
+
+ return;
+}
+
+#if 0
/* Called if we replace the cache on the object with a new
dbusmenu menuitem */
static void
@@ -175,6 +184,7 @@
return;
}
+#endif
/* Gets the positon of the child with its' parent if it has one.
Returns -1 if the position is unable to be calculated. */
@@ -213,8 +223,9 @@
ParserData *pdata = g_new0 (ParserData, 1);
g_object_set_data_full(G_OBJECT(item), PARSER_DATA, pdata, parse_data_free);
- g_object_set_data_full(G_OBJECT(widget), CACHED_MENUITEM, item, object_cache_freed);
+ /* g_object_set_data_full(G_OBJECT(widget), CACHED_MENUITEM, item, object_cache_freed); */
g_object_weak_ref(G_OBJECT(item), dbusmenu_item_freed, NULL);
+ g_object_weak_ref(G_OBJECT(widget), widget_freed, NULL);
pdata->widget = widget;
g_object_add_weak_pointer(G_OBJECT (widget), (gpointer*)&pdata->widget);
@@ -310,10 +321,6 @@
/* Oops, let's tell our parents about us */
if (peek == NULL) {
- /* TODO: Should we set a weak ref on the parent? */
- g_object_set_data (G_OBJECT (thisitem),
- "dbusmenu-parent",
- recurse->parent);
gint pos = get_child_position (widget);
if (pos >= 0)
dbusmenu_menuitem_child_add_position (recurse->parent,
@@ -802,7 +809,7 @@
G_CALLBACK (widget_notify_cb),
child);
- DbusmenuMenuitem *parent = g_object_get_data (G_OBJECT (child), "dbusmenu-parent");
+ DbusmenuMenuitem *parent = dbusmenu_menuitem_get_parent (child);
if (DBUSMENU_IS_MENUITEM (parent) && DBUSMENU_IS_MENUITEM (child))
{
Follow ups