GNOME Bugzilla – Bug 643611
Leak on gail_widget_get_description
Last modified: 2012-09-12 19:19:59 UTC
gail_widget_get_description uses as default description the widget tooltip, and it uses gtk-widget-get-tooltip-text to get it, in this way: return gtk_widget_get_tooltip_text (widget); But seeing the documentation of this method: Returns : the tooltip text, or NULL. You should free the returned string with g_free() when done. And taking into account that the description of atk_object_get_description is: const gchar* atk_object_get_description (AtkObject *accessible); is using const, atk users would not free it. That means that a new gchar is created but no one is freeing it => leak. An option would maintain a copy of the description as part of a private structure on GailWidget, and return this one. This should be updated when the tooltip_text property on the widget changes.
Yeah, I've noticed that as well in the drop-gail branch; I was planning to fix this and a couple of similar leaks during the drop-gail work.
*** Bug 683317 has been marked as a duplicate of this bug. ***
Created attachment 224044 [details] [review] Fixes the bug This patch solves the bug as I described on the comment 0. It caches the tooltip-text, and returns it instead of a new allocated string. It adds a private struct to GtkWidgetAccessible. Eventually layer should be moved there. I didn't do that as it is somewhat off-topic for this bug, and that layer is modified in other gtk accessible objects (so some getters/setters should be required).
Review of attachment 224044 [details] [review]: I would personally just use g_object_set_data here - makes the cleanup on finalize happen automatically...
Created attachment 224095 [details] [review] Fixes the bug Using g_object_[set/get]_data instead of adding a private structure.
Review of attachment 224095 [details] [review]: With the fix below, looks good ::: gtk/a11y/gtkwidgetaccessible.c @@ +121,3 @@ + g_object_set_data (G_OBJECT (accessible), + TOOLTIP_KEY, + gtk_widget_get_tooltip_text (widget)); No need to manually free, if you use g_object_set_data_full (..., g_free). And, in fact, you have to do that, otherwise you will leak the string at finalize time. The free function is used there too
(In reply to comment #6) > Review of attachment 224095 [details] [review]: > > With the fix below, looks good > > ::: gtk/a11y/gtkwidgetaccessible.c > @@ +121,3 @@ > + g_object_set_data (G_OBJECT (accessible), > + TOOLTIP_KEY, > + gtk_widget_get_tooltip_text (widget)); > > No need to manually free, if you use g_object_set_data_full (..., g_free). > > And, in fact, you have to do that, otherwise you will leak the string at > finalize time. The free function is used there too True. Sorry, I shouln't do this king of patches in a rush. Will update it soon.
(In reply to comment #6) > Review of attachment 224095 [details] [review]: > > With the fix below, looks good > > ::: gtk/a11y/gtkwidgetaccessible.c > @@ +121,3 @@ > + g_object_set_data (G_OBJECT (accessible), > + TOOLTIP_KEY, > + gtk_widget_get_tooltip_text (widget)); > > No need to manually free, if you use g_object_set_data_full (..., g_free). > > And, in fact, you have to do that, otherwise you will leak the string at > finalize time. The free function is used there too Committed after change g_object_set_data for g_object_set_data_full