From 37fb090b4f9fb1e1a894c5c57e7c3d28eb71f92d Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Thu, 24 Feb 2011 15:43:25 +0000 Subject: Hold a weak pointer to each DbusmenuMenuitems parent. This avoids using g_object_{get/set}_data for storing a nodes parent in the parser code, and the associated problems with that (ie, a child out-living its parent, leading to invalid reads --- libdbusmenu-glib/menuitem.c | 102 +++++++++++++++++++++++++++++++++++++++++++- libdbusmenu-glib/menuitem.h | 4 ++ libdbusmenu-gtk/parser.c | 6 +-- 3 files changed, 105 insertions(+), 7 deletions(-) diff --git a/libdbusmenu-glib/menuitem.c b/libdbusmenu-glib/menuitem.c index 89b2b91..49b83ce 100644 --- a/libdbusmenu-glib/menuitem.c +++ b/libdbusmenu-glib/menuitem.c @@ -62,6 +62,7 @@ struct _DbusmenuMenuitemPrivate gboolean realized; DbusmenuDefaults * defaults; gboolean exposed; + DbusmenuMenuitem * parent; }; /* Signals */ @@ -339,6 +340,11 @@ dbusmenu_menuitem_dispose (GObject *object) 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 @@ dbusmenu_menuitem_get_children (DbusmenuMenuitem * mi) /* 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 @@ dbusmenu_menuitem_take_children (DbusmenuMenuitem * mi) 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 @@ dbusmenu_menuitem_child_append (DbusmenuMenuitem * mi, DbusmenuMenuitem * child) 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 @@ dbusmenu_menuitem_child_prepend (DbusmenuMenuitem * mi, DbusmenuMenuitem * child 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 @@ dbusmenu_menuitem_child_delete (DbusmenuMenuitem * mi, DbusmenuMenuitem * child) 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 @@ dbusmenu_menuitem_child_add_position (DbusmenuMenuitem * mi, DbusmenuMenuitem * 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); } @@ -936,6 +956,84 @@ dbusmenu_menuitem_find_id (DbusmenuMenuitem * mi, gint id) return find_id.mi; } +/** + * 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. diff --git a/libdbusmenu-glib/menuitem.h b/libdbusmenu-glib/menuitem.h index 4a1c156..5f56803 100644 --- a/libdbusmenu-glib/menuitem.h +++ b/libdbusmenu-glib/menuitem.h @@ -379,6 +379,10 @@ gboolean dbusmenu_menuitem_child_reorder (DbusmenuMenuitem * mi, DbusmenuMenuite 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); diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 09b1e6e..44ea301 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -321,10 +321,6 @@ parse_menu_structure_helper (GtkWidget * widget, RecurseContext * recurse) /* 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, @@ -813,7 +809,7 @@ widget_notify_cb (GtkWidget *widget, 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)) { -- cgit v1.2.3 From 3cb0a087b79ef0af68b3ba73944f503823603bff Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Thu, 24 Feb 2011 10:26:27 -0600 Subject: Checking to ensure we're the parent before deleting --- libdbusmenu-glib/menuitem.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libdbusmenu-glib/menuitem.c b/libdbusmenu-glib/menuitem.c index 49b83ce..0bbce8c 100644 --- a/libdbusmenu-glib/menuitem.c +++ b/libdbusmenu-glib/menuitem.c @@ -781,6 +781,11 @@ dbusmenu_menuitem_child_delete (DbusmenuMenuitem * mi, DbusmenuMenuitem * child) g_return_val_if_fail(DBUSMENU_IS_MENUITEM(mi), FALSE); g_return_val_if_fail(DBUSMENU_IS_MENUITEM(child), FALSE); + if (dbusmenu_menuitem_get_parent(child) != mi) { + g_warning("Trying to remove a child that doesn't believe we're it's parent."); + return FALSE; + } + DbusmenuMenuitemPrivate * priv = DBUSMENU_MENUITEM_GET_PRIVATE(mi); priv->children = g_list_remove(priv->children, child); dbusmenu_menuitem_unparent(child); -- cgit v1.2.3 From 2dce5a22fac21e608b85630a825c36fc86c1c592 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Thu, 24 Feb 2011 10:27:07 -0600 Subject: Adding a transfer annotation for the get_parent function --- libdbusmenu-glib/menuitem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libdbusmenu-glib/menuitem.c b/libdbusmenu-glib/menuitem.c index 0bbce8c..de8a1ad 100644 --- a/libdbusmenu-glib/menuitem.c +++ b/libdbusmenu-glib/menuitem.c @@ -1028,7 +1028,7 @@ dbusmenu_menuitem_unparent (DbusmenuMenuitem * mi) * * This function looks up the parent of @mi * - * Return value: The parent of this menu item + * Return value: (transfer none): The parent of this menu item */ DbusmenuMenuitem * dbusmenu_menuitem_get_parent (DbusmenuMenuitem * mi) -- cgit v1.2.3 From 233e2b72456b70ea86c163f25c08d5fa1981b42c Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Thu, 24 Feb 2011 10:32:35 -0600 Subject: Add the new functions to the sections doc --- docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt b/docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt index b6a6902..b3a78b7 100644 --- a/docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt +++ b/docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt @@ -100,6 +100,9 @@ dbusmenu_menuitem_foreach dbusmenu_menuitem_handle_event dbusmenu_menuitem_send_about_to_show dbusmenu_menuitem_show_to_user +dbusmenu_menuitem_get_parent +dbusmenu_menuitem_set_parent +dbusmenu_menuitem_unparent DBUSMENU_MENUITEM DBUSMENU_IS_MENUITEM -- cgit v1.2.3