GNOME.org
 

[nautilus-actions] Fix a memory leak when removing items from the tree store

commit 76ed9812cb3b898e14466c70ff765272c14d8a9c
Author: Pierre Wieser <pwieser trychlos org>
Date:   Sun Sep 27 00:23:20 2009 +0200

    Fix a memory leak when removing items from the tree store

 src/common/na-object-class.h  |    1 -
 src/common/na-object-item.c   |    8 +++--
 src/common/na-object.c        |    1 -
 src/nact/nact-iactions-list.c |   32 +++++++++++----------
 src/nact/nact-iactions-list.h |    2 +-
 src/nact/nact-main-menubar.c  |    8 ++++-
 src/nact/nact-main-window.c   |    9 ++++++
 src/nact/nact-tree-model.c    |   63 ++++++++++++++++++++++++----------------
 src/nact/nact-tree-model.h    |   24 ++++++++--------
 9 files changed, 88 insertions(+), 60 deletions(-)
---
diff --git a/src/common/na-object-class.h b/src/common/na-object-class.h
index 5d0b018..07506b8 100644
--- a/src/common/na-object-class.h
+++ b/src/common/na-object-class.h
@@ -97,7 +97,6 @@ typedef struct {
 	gchar *    ( *get_clipboard_id )( const NAObject *object );
 
 	/**
-	 * TODO: get ride of this
 	 * ref:
 	 * @object: the #NAObject-derived object to be reffed.
 	 *
diff --git a/src/common/na-object-item.c b/src/common/na-object-item.c
index 9053233..211d026 100644
--- a/src/common/na-object-item.c
+++ b/src/common/na-object-item.c
@@ -589,6 +589,8 @@ na_object_item_free_items( GList *items )
 	for( it = items ; it ; it = it->next ){
 		if( G_IS_OBJECT( it->data )){
 			g_object_unref( it->data );
+		} else {
+			g_warning( "na_object_item_free_items: %p not an object", ( void * ) it->data );
 		}
 	}
 
@@ -734,7 +736,7 @@ na_object_item_set_items( NAObjectItem *item, GList *items )
  *
  * Appends a new @object to the list of subitems of @item.
  *
- * We add a reference on provided @object.
+ * Doesn't modify the reference count on @object.
  */
 void
 na_object_item_append_item( NAObjectItem *item, const NAObject *object )
@@ -758,7 +760,7 @@ na_object_item_append_item( NAObjectItem *item, const NAObject *object )
  *
  * Inserts a new @object in the list of subitems of @item.
  *
- * We add a reference on provided @object.
+ * Doesn't modify the reference count on @object.
  */
 void
 na_object_item_insert_item( NAObjectItem *item, const NAObject *object, const NAObject *before )
@@ -789,7 +791,7 @@ na_object_item_insert_item( NAObjectItem *item, const NAObject *object, const NA
  *
  * Removes an @object from the list of subitems of @item.
  *
- * We decrement the reference count on @object.
+ * Doesn't modify the reference count on @object.
  */
 void
 na_object_item_remove_item( NAObjectItem *item, const NAObject *object )
diff --git a/src/common/na-object.c b/src/common/na-object.c
index 732973d..ac6ceda 100644
--- a/src/common/na-object.c
+++ b/src/common/na-object.c
@@ -543,7 +543,6 @@ na_object_object_get_clipboard_id( const NAObject *object )
 }
 
 /**
- * TODO: get ride of this
  * na_object_object_ref:
  * @object: the #NAObject-derived object to be reffed.
  *
diff --git a/src/nact/nact-iactions-list.c b/src/nact/nact-iactions-list.c
index 05bae1b..cbda05d 100644
--- a/src/nact/nact-iactions-list.c
+++ b/src/nact/nact-iactions-list.c
@@ -263,6 +263,7 @@ interface_base_init( NactIActionsListInterface *klass )
 static void
 free_items_callback( NactIActionsList *instance, GList *items )
 {
+	g_debug( "free_items_callback: selection=%p (%d items)", ( void * ) items, g_list_length( items ));
 	na_object_free_items( items );
 }
 
@@ -482,45 +483,43 @@ nact_iactions_list_dispose( NactIActionsList *instance )
 }
 
 /**
- * nact_iactions_list_delete_selection:
+ * nact_iactions_list_delete:
  * @window: this #NactIActionsList instance.
+ * @list: list of #NAObject to be deleted.
  *
- * Deletes the current selection from the underlying tree store.
+ * Deletes the specified list from the underlying tree store.
  *
  * This function takes care of repositionning a new selection if
  * possible, and refilter the display model.
  */
 void
-nact_iactions_list_delete_selection( NactIActionsList *instance )
+nact_iactions_list_delete( NactIActionsList *instance, GList *items )
 {
 	GtkTreeView *treeview;
 	GtkTreeModel *model;
-	GtkTreeSelection *selection;
-	GList *selected;
 	GtkTreePath *path = NULL;
+	GList *it;
 
 	g_return_if_fail( NACT_IS_IACTIONS_LIST( instance ));
 
 	if( st_initialized && !st_finalized ){
 
 		treeview = get_actions_list_treeview( instance );
-		selection = gtk_tree_view_get_selection( treeview );
-		selected = gtk_tree_selection_get_selected_rows( selection, &model );
+		model = gtk_tree_view_get_model( treeview );
 
 		set_selection_changed_mode( instance, FALSE );
 
-		if( g_list_length( selected )){
-			path = gtk_tree_path_copy(( GtkTreePath * ) selected->data );
-			nact_tree_model_remove( NACT_TREE_MODEL( model ), selected );
+		for( it = items ; it ; it = it->next ){
+			if( path ){
+				gtk_tree_path_free( path );
+			}
+			path = nact_tree_model_remove( NACT_TREE_MODEL( model ), NA_OBJECT( it->data ));
 		}
 
 		set_selection_changed_mode( instance, TRUE );
-
-		g_list_foreach( selected, ( GFunc ) gtk_tree_path_free, NULL );
-		g_list_free( selected );
+		gtk_tree_model_filter_refilter( GTK_TREE_MODEL_FILTER( model ));
 
 		if( path ){
-			gtk_tree_model_filter_refilter( GTK_TREE_MODEL_FILTER( model ));
 			select_row_at_path( instance, treeview, model, path );
 			gtk_tree_path_free( path );
 		}
@@ -1431,10 +1430,13 @@ on_treeview_selection_changed( GtkTreeSelection *selection, NactIActionsList *in
 	GList *selected_items;
 
 	selected_items = nact_iactions_list_get_selected_items( instance );
+	g_debug( "on_treeview_selection_changed: selection=%p (%d items)", ( void * ) selected_items, g_list_length( selected_items ));
 
 	if( is_selection_changed_authorized( instance )){
 		g_signal_emit_by_name( instance, IACTIONS_LIST_SIGNAL_SELECTION_CHANGED, selected_items );
 	}
+
+	/* selection list if free in cleanup handler for the signal */
 }
 
 /*
@@ -1512,7 +1514,7 @@ select_first_row( NactIActionsList *instance )
  * @window: this #NactIActionsList instance.
  * @path: a #GtkTreePath.
  *
- * Select the rows at the required path, or the next following, or
+ * Select the row at the required path, or the next following, or
  * the immediate previous.
  */
 static void
diff --git a/src/nact/nact-iactions-list.h b/src/nact/nact-iactions-list.h
index f1e4628..accf080 100644
--- a/src/nact/nact-iactions-list.h
+++ b/src/nact/nact-iactions-list.h
@@ -94,7 +94,7 @@ void      nact_iactions_list_runtime_init_toplevel( NactIActionsList *instance,
 void      nact_iactions_list_all_widgets_showed( NactIActionsList *instance );
 void      nact_iactions_list_dispose( NactIActionsList *instance );
 
-void      nact_iactions_list_delete_selection( NactIActionsList *instance );
+void      nact_iactions_list_delete( NactIActionsList *instance, GList *items );
 void      nact_iactions_list_fill( NactIActionsList *instance, GList *items );
 NAObject *nact_iactions_list_get_item( NactIActionsList *instance, const gchar *uuid );
 GList    *nact_iactions_list_get_items( NactIActionsList *instance );
diff --git a/src/nact/nact-main-menubar.c b/src/nact/nact-main-menubar.c
index ec79c71..3ddfbfd 100644
--- a/src/nact/nact-main-menubar.c
+++ b/src/nact/nact-main-menubar.c
@@ -489,7 +489,7 @@ on_cut_activated( GtkAction *gtk_action, NactMainWindow *window )
 	items = nact_iactions_list_get_selected_items( NACT_IACTIONS_LIST( window ));
 	nact_main_window_move_to_deleted( window, items );
 	nact_clipboard_primary_set( items, FALSE );
-	nact_iactions_list_delete_selection( NACT_IACTIONS_LIST( window ));
+	nact_iactions_list_delete( NACT_IACTIONS_LIST( window ), items );
 
 	/* do not unref selected items as the ref has been moved to main_deleted
 	 */
@@ -584,13 +584,17 @@ static void
 on_delete_activated( GtkAction *gtk_action, NactMainWindow *window )
 {
 	GList *items;
+	GList *it;
 
 	g_return_if_fail( GTK_IS_ACTION( gtk_action ));
 	g_return_if_fail( NACT_IS_MAIN_WINDOW( window ));
 
 	items = nact_iactions_list_get_selected_items( NACT_IACTIONS_LIST( window ));
+	for( it = items ; it ; it = it->next ){
+		g_debug( "on_delete_activated: items=%p (%s)", ( void * ) it->data, G_OBJECT_TYPE_NAME( it->data ));
+	}
 	nact_main_window_move_to_deleted( window, items );
-	nact_iactions_list_delete_selection( NACT_IACTIONS_LIST( window ));
+	nact_iactions_list_delete( NACT_IACTIONS_LIST( window ), items );
 
 	/* do not unref selected items as the ref has been moved to main_deleted
 	 */
diff --git a/src/nact/nact-main-window.c b/src/nact/nact-main-window.c
index ce652e8..d535e9c 100644
--- a/src/nact/nact-main-window.c
+++ b/src/nact/nact-main-window.c
@@ -492,6 +492,7 @@ instance_dispose( GObject *window )
 	NactMainWindow *self;
 	GtkWidget *pane;
 	gint pos;
+	GList *it;
 
 	g_debug( "%s: window=%p", thisfn, ( void * ) window );
 	g_return_if_fail( NACT_IS_MAIN_WINDOW( window ));
@@ -507,6 +508,9 @@ instance_dispose( GObject *window )
 		pos = gtk_paned_get_position( GTK_PANED( pane ));
 		base_iprefs_set_int( BASE_WINDOW( window ), "main-paned", pos );
 
+		for( it = self->private->deleted ; it ; it = it->next ){
+			g_debug( "nact_main_window_instance_dispose: %p (%s)", ( void * ) it->data, G_OBJECT_TYPE_NAME( it->data ));
+		}
 		na_object_free_items( self->private->deleted );
 
 		nact_iactions_list_dispose( NACT_IACTIONS_LIST( window ));
@@ -647,10 +651,15 @@ nact_main_window_has_modified_items( const NactMainWindow *window )
 void
 nact_main_window_move_to_deleted( NactMainWindow *window, GList *items )
 {
+	GList *it;
+
 	g_return_if_fail( NACT_IS_MAIN_WINDOW( window ));
 
 	if( !window->private->dispose_has_run ){
 		window->private->deleted = g_list_concat( window->private->deleted, items );
+		for( it = window->private->deleted ; it ; it = it->next ){
+			g_debug( "nact_main_window_move_to_deleted: %p (%s)", ( void * ) it->data, G_OBJECT_TYPE_NAME( it->data ));
+		}
 	}
 }
 
diff --git a/src/nact/nact-tree-model.c b/src/nact/nact-tree-model.c
index 045c579..b1231e9 100644
--- a/src/nact/nact-tree-model.c
+++ b/src/nact/nact-tree-model.c
@@ -164,12 +164,13 @@ static void           insert_before_parent_get_iters( GtkTreeModel *model,  GtkT
 static void           insert_as_last_child_get_iters( GtkTreeModel *model,  GtkTreePath *select_path, const NAObject *object, GtkTreeIter *parent_iter, gboolean *has_parent_iter, GtkTreeIter *sibling_iter, gboolean *has_sibling_iter, NAObject **parent_object );
 static void           remove_if_exists( NactTreeModel *model, GtkTreeModel *store, const NAObject *object );
 
-static GList         *add_parent( GList *parents, GtkTreeModel *store, GtkTreeIter *obj_iter, GtkTreePath *obj_path );
+static GList         *add_parent( GList *parents, GtkTreeModel *store, GtkTreeIter *obj_iter );
 static void           append_item( GtkTreeStore *model, GtkTreeView *treeview, GtkTreeIter *parent, GtkTreeIter *iter, const NAObject *object );
 static void           display_item( GtkTreeStore *model, GtkTreeView *treeview, GtkTreeIter *iter, const NAObject *object );
 static gboolean       dump_store( NactTreeModel *model, GtkTreePath *path, NAObject *object, ntmDumpStruct *ntm );
 static void           iter_on_store( NactTreeModel *model, GtkTreeModel *store, GtkTreeIter *parent, FnIterOnStore fn, gpointer user_data );
 static gboolean       iter_on_store_item( NactTreeModel *model, GtkTreeModel *store, GtkTreeIter *iter, FnIterOnStore fn, gpointer user_data );
+static gboolean       remove_items( GtkTreeStore *store, GtkTreeIter *iter );
 static gboolean       search_for_object( NactTreeModel *model, GtkTreeModel *store, const NAObject *object, GtkTreeIter *iter );
 static gboolean       search_for_objet_iter( NactTreeModel *model, GtkTreePath *path, NAObject *object, ntmSearchStruct *ntm );
 static gboolean       search_for_object_id( NactTreeModel *model, GtkTreeModel *store, const NAObject *object, GtkTreeIter *iter );
@@ -936,54 +937,48 @@ nact_tree_model_iter( NactTreeModel *model, FnIterOnStore fn, gpointer user_data
 /**
  * nact_tree_model_remove:
  * @model: this #NactTreeModel instance.
- * @selected: a list of #GtkTreePath as returned by
- * gtk_tree_selection_get_selected_rows().
+ * @object: the #NAObject to be deleted.
  *
- * Deletes the selected rows, unref-ing the underlying objects if any.
+ * Recursively deletes the specified object.
  *
- * We begin by the end, so that we are almost sure that path remain
- * valid during the iteration.
+ * Returns: a path which may be suitable for the next selection.
  */
-void
-nact_tree_model_remove( NactTreeModel *model, GList *selected )
+GtkTreePath *
+nact_tree_model_remove( NactTreeModel *model, NAObject *object )
 {
-	GList *reversed, *it;
 	GtkTreeIter iter;
 	GtkTreeStore *store;
-	gchar *path_str;
 	GList *parents = NULL;
+	GList *it;
+	GtkTreePath *path = NULL;
 
-	g_return_if_fail( NACT_IS_TREE_MODEL( model ));
+	g_return_val_if_fail( NACT_IS_TREE_MODEL( model ), NULL );
 
 	if( !model->private->dispose_has_run ){
 
-		reversed = g_list_reverse( selected );
 		store = GTK_TREE_STORE( gtk_tree_model_filter_get_model( GTK_TREE_MODEL_FILTER( model )));
 
-		for( it = reversed ; it ; it = it->next ){
-
-			path_str = gtk_tree_path_to_string(( GtkTreePath * ) it->data );
-			g_debug( "nact_tree_model_remove: path=%s", path_str );
-			g_free( path_str );
-
-			gtk_tree_model_get_iter( GTK_TREE_MODEL( store ), &iter, ( GtkTreePath * ) it->data );
-			parents = add_parent( parents, GTK_TREE_MODEL( store ), &iter, ( GtkTreePath * ) it->data );
-			gtk_tree_store_remove( store, &iter );
+		if( search_for_object( model, GTK_TREE_MODEL( store ), object, &iter )){
+			parents = add_parent( parents, GTK_TREE_MODEL( store ), &iter );
+			path = gtk_tree_model_get_path( GTK_TREE_MODEL( store ), &iter );
+			remove_items( store, &iter );
 		}
 
 		for( it = parents ; it ; it = it->next ){
 			na_object_check_edition_status( it->data );
 		}
 	}
+
+	return( path );
 }
 
 /*
- * iter and path are positionned on the row which is going to be deleted
+ * iter is positionned on the row which is going to be deleted
  * remove the object from the subitems list of parent (if any)
  * add parent to the list to check its status after remove will be done
  */
 static GList *
-add_parent( GList *parents, GtkTreeModel *store, GtkTreeIter *obj_iter, GtkTreePath *obj_path )
+add_parent( GList *parents, GtkTreeModel *store, GtkTreeIter *obj_iter )
 {
 	NAObject *object;
 	GtkTreePath *path;
@@ -991,7 +986,7 @@ add_parent( GList *parents, GtkTreeModel *store, GtkTreeIter *obj_iter, GtkTreeP
 	NAObject *parent;
 
 	gtk_tree_model_get( store, obj_iter, IACTIONS_LIST_NAOBJECT_COLUMN, &object, -1 );
-	path = gtk_tree_path_copy( obj_path );
+	path = gtk_tree_model_get_path( store, obj_iter );
 
 	if( gtk_tree_path_get_depth( path ) > 1 ){
 		gtk_tree_path_up( path );
@@ -1098,6 +1093,24 @@ iter_on_store_item( NactTreeModel *model, GtkTreeModel *store, GtkTreeIter *iter
 	return( stop );
 }
 
+/*
+ * recursively remove child items starting with iter
+ * returns TRUE if iter is always valid after the remove
+ */
+static gboolean
+remove_items( GtkTreeStore *store, GtkTreeIter *iter )
+{
+	GtkTreeIter child;
+	gboolean valid;
+
+	while( gtk_tree_model_iter_children( GTK_TREE_MODEL( store ), &child, iter )){
+		remove_items( store, &child );
+	}
+	valid = gtk_tree_store_remove( store, iter );
+
+	return( valid );
+}
+
 static gboolean
 search_for_object( NactTreeModel *model, GtkTreeModel *store, const NAObject *object, GtkTreeIter *result_iter )
 {
@@ -1303,7 +1316,7 @@ imulti_drag_source_drag_data_get( EggTreeMultiDragSource *drag_source,
 			break;
 	}
 
-	g_list_free( selected_items );
+	na_object_free_items( selected_items );
 	return( ret );
 }
 
diff --git a/src/nact/nact-tree-model.h b/src/nact/nact-tree-model.h
index 1ac9009..c4ca015 100644
--- a/src/nact/nact-tree-model.h
+++ b/src/nact/nact-tree-model.h
@@ -87,18 +87,18 @@ enum {
  */
 typedef gboolean ( *FnIterOnStore )( NactTreeModel *, GtkTreePath *, NAObject *, gpointer );
 
-GType   nact_tree_model_get_type( void );
-
-void    nact_tree_model_initial_load( BaseWindow *window, GtkTreeView *treeview );
-void    nact_tree_model_runtime_init( NactTreeModel *model, gboolean have_dnd );
-void    nact_tree_model_dispose( NactTreeModel *model );
-
-void    nact_tree_model_display( NactTreeModel *model, NAObject *object );
-void    nact_tree_model_dump( NactTreeModel *model );
-void    nact_tree_model_fill( NactTreeModel *model, GList *items, gboolean only_actions);
-gchar  *nact_tree_model_insert( NactTreeModel *model, const NAObject *object, GtkTreePath *path, NAObject **parent );
-void    nact_tree_model_iter( NactTreeModel *model, FnIterOnStore fn, gpointer user_data );
-void    nact_tree_model_remove( NactTreeModel *model, GList *selected );
+GType        nact_tree_model_get_type( void );
+
+void         nact_tree_model_initial_load( BaseWindow *window, GtkTreeView *treeview );
+void         nact_tree_model_runtime_init( NactTreeModel *model, gboolean have_dnd );
+void         nact_tree_model_dispose( NactTreeModel *model );
+
+void         nact_tree_model_display( NactTreeModel *model, NAObject *object );
+void         nact_tree_model_dump( NactTreeModel *model );
+void         nact_tree_model_fill( NactTreeModel *model, GList *items, gboolean only_actions);
+gchar       *nact_tree_model_insert( NactTreeModel *model, const NAObject *object, GtkTreePath *path, NAObject **parent );
+void         nact_tree_model_iter( NactTreeModel *model, FnIterOnStore fn, gpointer user_data );
+GtkTreePath *nact_tree_model_remove( NactTreeModel *model, NAObject *object );
 
 G_END_DECLS
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]