[grisbi-devel] [PATCH 08/15] Use proper ref counting to free transaction_model's CustomList

RĂ©mi Cardona remi at gentoo.org
Sat Mar 9 16:54:00 CET 2013


The tree widget in the main window holds a reference to the transaction
model but (the horribly named) transaction_model_initialize() would free
up everything inside it.

Since CustomList _is_ a fully-fledged GObject, using its reference count
seems like an obvious solution to invalid memory errors (caught using
valgrind).
---
 src/custom_list.c          | 40 +++++++++++++++++++++++++++++-
 src/traitement_variables.c |  2 +-
 src/transaction_list.c     |  1 +
 src/transaction_model.c    | 62 +++++-----------------------------------------
 src/transaction_model.h    |  1 -
 5 files changed, 47 insertions(+), 59 deletions(-)

diff --git a/src/custom_list.c b/src/custom_list.c
index c2d8058..129937a 100644
--- a/src/custom_list.c
+++ b/src/custom_list.c
@@ -247,7 +247,45 @@ static void custom_list_init (CustomList *custom_list)
  * */
 void custom_list_finalize (GObject *object)
 {
-    transaction_model_initialize ();
+    CustomList *custom_list;
+    gint i;
+
+    custom_list = CUSTOM_LIST ( object );
+
+    /* free all records and free all memory used by the list */
+    for (i=0 ; i<custom_list -> num_rows ; i++)
+    {
+	CustomRecord *record;
+	gint j;
+
+	record = custom_list -> rows[i];
+	for (j=0 ; j<CUSTOM_MODEL_VISIBLE_COLUMNS ; j++)
+	    if (record -> visible_col[j])
+		g_free (record -> visible_col[j]);
+
+	if (record -> number_of_children)
+	{
+	    gint k;
+	    for (k=0 ; k<record -> number_of_children ; k++)
+	    {
+		CustomRecord *child_record;
+
+		child_record = record -> children_rows[k];
+
+		for (j=0 ; j<CUSTOM_MODEL_VISIBLE_COLUMNS ; j++)
+		    if (child_record -> visible_col[j])
+			g_free (child_record -> visible_col[j]);
+		g_free (child_record);
+	    }
+	    g_free (record -> children_rows);
+
+	    for (k=0 ; k<TRANSACTION_LIST_ROWS_NB ; k++)
+		record -> transaction_records[k] -> number_of_children = 0;
+	}
+	g_free (record);
+    }
+    g_free (custom_list -> rows);
+    g_free (custom_list -> visibles_rows);
 
     /* must chain up - finalize parent */
     G_OBJECT_CLASS(custom_list_parent_class)->finalize (object);
diff --git a/src/traitement_variables.c b/src/traitement_variables.c
index e4baa2e..140e727 100644
--- a/src/traitement_variables.c
+++ b/src/traitement_variables.c
@@ -157,7 +157,7 @@ void init_variables ( void )
 
     /* if ever there is still something from the previous list,
      * erase now */
-    transaction_model_initialize();
+    transaction_model_set_model ( NULL );
 
     gsb_data_account_init_variables ();
     gsb_data_transaction_init_variables ();
diff --git a/src/transaction_list.c b/src/transaction_list.c
index fbbfeae..5e6e842 100644
--- a/src/transaction_list.c
+++ b/src/transaction_list.c
@@ -97,6 +97,7 @@ gboolean transaction_list_create (void)
 
     custom_list = custom_list_new ();
     transaction_model_set_model (custom_list);
+    g_object_unref ( custom_list );
 
     return (custom_list != NULL);
 }
diff --git a/src/transaction_model.c b/src/transaction_model.c
index abbb17d..f94452e 100644
--- a/src/transaction_model.c
+++ b/src/transaction_model.c
@@ -56,62 +56,6 @@ static CustomList *custom_list = NULL;
 
 
 /**
- * erase the custom_list variable and free the memory used by the list
- * called when closing a file, while destroying the list
- *
- * \param
- *
- * \return
- * */
-void transaction_model_initialize ( void )
-{
-    gint i;
-
-    if (!custom_list)
-	return;
-
-    /* free all records and free all memory used by the list */
-    for (i=0 ; i<custom_list -> num_rows ; i++)
-    {
-	CustomRecord *record;
-	gint j;
-
-	record = custom_list -> rows[i];
-	for (j=0 ; j<CUSTOM_MODEL_VISIBLE_COLUMNS ; j++)
-	    if (record -> visible_col[j])
-		g_free (record -> visible_col[j]);
-
-	if (record -> number_of_children)
-	{
-	    gint k;
-	    for (k=0 ; k<record -> number_of_children ; k++)
-	    {
-		CustomRecord *child_record;
-
-		child_record = record -> children_rows[k];
-
-		for (j=0 ; j<CUSTOM_MODEL_VISIBLE_COLUMNS ; j++)
-		    if (child_record -> visible_col[j])
-			g_free (child_record -> visible_col[j]);
-		g_free (child_record);
-	    }
-	    g_free (record -> children_rows);
-
-	    for (k=0 ; k<TRANSACTION_LIST_ROWS_NB ; k++)
-		record -> transaction_records[k] -> number_of_children = 0;
-	}
-	g_free (record);
-    }
-    g_free (custom_list -> rows);
-    g_free (custom_list -> visibles_rows);
-
-    /* cannot free custom_list without crashing grisbi... perhaps done automatically
-     * when destroying the tree view ? */
-    /*     g_free (custom_list); */
-    custom_list = NULL;
-}
-
-/**
  * return the CustomList
  *
  * \param
@@ -132,7 +76,13 @@ CustomList *transaction_model_get_model (void)
  * */
 void transaction_model_set_model ( CustomList *new_custom_list )
 {
+    if ( custom_list )
+        g_object_unref ( G_OBJECT ( custom_list ) );
+
     custom_list = new_custom_list;
+
+    if ( custom_list )
+        g_object_ref ( G_OBJECT ( custom_list ) );
 }
 
 
diff --git a/src/transaction_model.h b/src/transaction_model.h
index dd1f64e..119c393 100644
--- a/src/transaction_model.h
+++ b/src/transaction_model.h
@@ -13,7 +13,6 @@ CustomList *transaction_model_get_model (void);
 gboolean transaction_model_get_transaction_iter ( GtkTreeIter *iter,
 						  gint transaction_number,
 						  gint line_in_transaction );
-void transaction_model_initialize ( void );
 gboolean transaction_model_iter_next (GtkTreeIter *iter);
 void transaction_model_set_model ( CustomList *new_custom_list );
 /* END_DECLARATION */
-- 
1.8.1.4



More information about the devel mailing list