>From 17936ae138026e045c1a4f6c42eaa288405ac528 Mon Sep 17 00:00:00 2001
From: Andreas Winkelmann <ml awinkelmann de>
Date: Sat, 24 May 2014 01:44:45 +0200
Subject: [PATCH] Several fixes and improvements regarding
 Album/Artist/File-Lists

Memory Leak, The "All Albums" Node in the Album-List has a List attached which was not free()d.
Added a boolean column to the albumListModel to identify these nodes and clear()ed the attached Lists.

A crash which happened because the ArtistAlbumList was used after free()d. The List was freed in
ET_Free_Artist_Album_File_List() but Pointers to the Nodes were still attached to the artist /
albumListModels.

Another Crash at Application Shutdown because while freeing these Lists also Row-Changed Signals
were emitted. Disabled the Signals while freeing the Lists.

Removed g_object_unref()'s for the File, Artist and Album-List-Models. We are keeping the Pointers
to these Models and using them while the whole Application Lifetime so an unref is probably to early
here. Didn't found a better place.

Some additional Variable Initializations before gtk_tree_model_get() because it's possible the function
fails to set these variables.
---
 src/browser.c | 142 ++++++++++++++++++++++++++++++++++++++--------------------
 src/browser.h |   4 ++
 src/et_core.c |  31 ++++++++-----
 src/et_core.h |   2 +-
 4 files changed, 119 insertions(+), 60 deletions(-)

diff --git a/src/browser.c b/src/browser.c
index f80d90c..7a4d278 100644
--- a/src/browser.c
+++ b/src/browser.c
@@ -67,7 +67,7 @@ static GtkWidget    *BrowserLabel;
 static GtkWidget    *BrowserButton;
 static GtkWidget    *BrowserNoteBook;
 static GtkListStore *artistListModel;
-static GtkListStore *albumListModel;
+static GtkListStore *albumListModel = NULL;
 /* Path selected in the browser area (BrowserEntry or BrowserTree). */
 static gchar        *BrowserCurrentPath = NULL;
 
@@ -156,6 +156,10 @@ static void Browser_Album_List_Row_Selected (GtkTreeSelection *selection,
                                              gpointer data);
 static void Browser_Album_List_Set_Row_Appearance (GtkTreeIter *row);
 
+void Browser_Album_List_Clear( void );
+
+void Browser_Artist_List_Clear( void );
+
 static void Browser_Update_Current_Path (const gchar *path);
 
 #ifdef G_OS_WIN32
@@ -1188,17 +1192,13 @@ void Browser_List_Refresh_Whole_List (void)
         gtk_tree_path_free(currentPath);
 
 
-        for (row=0; row < gtk_tree_model_iter_n_children(GTK_TREE_MODEL(albumListModel), NULL); row++)
+        valid = gtk_tree_model_get_iter_first( GTK_TREE_MODEL(albumListModel), &iter );
+        while ( valid )
         {
-            if (row == 0)
-                currentPath = gtk_tree_path_new_first();
-            else
-                gtk_tree_path_next(currentPath);
-
-            gtk_tree_model_get_iter(GTK_TREE_MODEL(albumListModel), &iter, currentPath);
             Browser_Album_List_Set_Row_Appearance(&iter);
+
+        	valid = gtk_tree_model_iter_next( GTK_TREE_MODEL(albumListModel), &iter );
         }
-        gtk_tree_path_free(currentPath);
     }
 }
 
@@ -1219,10 +1219,10 @@ void Browser_List_Refresh_File_In_List (ET_File *ETFile)
     File_Tag  *FileTag;
     File_Name *FileName;
     gboolean row_found = FALSE;
+    gboolean valid;
     gchar *current_basename_utf8;
     gchar *track;
     gchar *disc;
-    gboolean valid;
     gint row;
     gchar *artist, *album;
 
@@ -1365,30 +1365,24 @@ void Browser_List_Refresh_File_In_List (ET_File *ETFile)
         //
         // FIX ME : see also if we must add a new line / or change list of the ETFile
         //
-        for (row=0; row < gtk_tree_model_iter_n_children(GTK_TREE_MODEL(albumListModel), NULL); row++)
-        {
-            if (row == 0)
-                currentPath = gtk_tree_path_new_first();
-            else
-                gtk_tree_path_next(currentPath);
-
-            valid = gtk_tree_model_get_iter(GTK_TREE_MODEL(albumListModel), &selectedIter, currentPath);
-            if (valid)
-            {
-                gtk_tree_model_get(GTK_TREE_MODEL(albumListModel), &selectedIter, ALBUM_NAME, &album, -1);
 
-                if ( (!current_album && !album)
-                ||   (current_album && album && g_utf8_collate(current_album,album)==0) )
-                {
-                    // Set color of the row
-                    Browser_Album_List_Set_Row_Appearance(&selectedIter);
-                    g_free(album);
-                    break;
-                }
-                g_free(album);
-            }
+        valid = gtk_tree_model_get_iter_first( GTK_TREE_MODEL(albumListModel), &selectedIter );
+        while ( valid )
+        {
+        	album = NULL;
+        	gtk_tree_model_get( GTK_TREE_MODEL(albumListModel), &selectedIter, ALBUM_NAME, &album, -1);
+
+        	if ( current_album && album && g_utf8_collate(current_album,album)==0)
+        	{
+        		// Set color of the row
+        		Browser_Album_List_Set_Row_Appearance(&selectedIter);
+        		g_free(album);
+        		break;
+        	}
+        	g_free(album);
+
+        	valid = gtk_tree_model_iter_next( GTK_TREE_MODEL(albumListModel), &selectedIter );
         }
-        gtk_tree_path_free(currentPath); currentPath = NULL;
 
         //
         // FIX ME : see also if we must add a new line / or change list of the ETFile
@@ -1726,9 +1720,8 @@ ET_File *Browser_List_Select_File_By_DLM (const gchar* string, gboolean select_i
 void Browser_List_Clear()
 {
     gtk_list_store_clear(fileListModel);
-    gtk_list_store_clear(artistListModel);
-    gtk_list_store_clear(albumListModel);
-
+    Browser_Artist_List_Clear();
+    Browser_Album_List_Clear();
 }
 
 /*
@@ -2127,11 +2120,11 @@ Browser_Album_List_Load_Files (GList *albumlist, ET_File *etfile_to_select)
     if (etfile_to_select)
         album_to_select = ((File_Tag *)etfile_to_select->FileTag->data)->album;
 
-    gtk_list_store_clear(albumListModel);
+    Browser_Album_List_Clear();
+
     selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(BrowserAlbumList));
 
     // Create a first row to select all albums of the artist
-    // FIX ME : the attached list must be freed!
     for (l = albumlist; l != NULL; l = g_list_next (l))
     {
         GList *etfilelist_tmp;
@@ -2142,11 +2135,12 @@ Browser_Album_List_Load_Files (GList *albumlist, ET_File *etfile_to_select)
         etfilelist = g_list_concat(etfilelist, etfilelist_tmp);
     }
 
-    gtk_list_store_insert_with_values (albumListModel, &iter, G_MAXINT,
+    gtk_list_store_insert_with_values (albumListModel, &iter, -1,
                                        ALBUM_NAME, _("<All albums>"),
                                        ALBUM_NUM_FILES,
-                                       g_list_length (g_list_first (etfilelist)),
+                                       g_list_length (etfilelist),
                                        ALBUM_ETFILE_LIST_POINTER, etfilelist,
+                                       ALBUM_ALL_ALBUMS_ROW, TRUE,
                                        -1);
 
     // Create a line for each album of the artist
@@ -2163,13 +2157,14 @@ Browser_Album_List_Load_Files (GList *albumlist, ET_File *etfile_to_select)
         icon = g_themed_icon_new_with_default_fallbacks ("media-optical-cd-audio");
 
         /* Add the new row. */
-        gtk_list_store_insert_with_values (albumListModel, &iter, G_MAXINT,
+        gtk_list_store_insert_with_values (albumListModel, &iter, -1,
                                            ALBUM_GICON, icon,
                                            ALBUM_NAME, albumname,
                                            ALBUM_NUM_FILES,
                                            g_list_length (g_list_first (etfilelist)),
-                                           ALBUM_ETFILE_LIST_POINTER,
-                                           etfilelist, -1);
+                                           ALBUM_ETFILE_LIST_POINTER, etfilelist,
+                                           ALBUM_ALL_ALBUMS_ROW, FALSE,
+                                           -1);
 
         g_object_unref (icon);
 
@@ -2219,7 +2214,7 @@ Browser_Album_List_Load_Files (GList *albumlist, ET_File *etfile_to_select)
 static void
 Browser_Album_List_Row_Selected (GtkTreeSelection *selection, gpointer data)
 {
-    GList *etfilelist;
+    GList *etfilelist = NULL;
     GtkTreeIter iter;
 
 
@@ -2240,6 +2235,7 @@ Browser_Album_List_Row_Selected (GtkTreeSelection *selection, gpointer data)
 
     // Displays the first item
     Action_Select_Nth_File_By_Etfile((ET_File *)etfilelist->data);
+
 }
 
 
@@ -2249,10 +2245,9 @@ Browser_Album_List_Row_Selected (GtkTreeSelection *selection, gpointer data)
 static void
 Browser_Album_List_Set_Row_Appearance (GtkTreeIter *iter)
 {
-    GList *l;
+    GList *l = NULL;
     gboolean not_all_saved = FALSE;
 
-
     // Change the style (red/bold) of the row if one of the files was changed
     for (gtk_tree_model_get (GTK_TREE_MODEL (albumListModel), iter,
                              ALBUM_ETFILE_LIST_POINTER, &l, -1);
@@ -2286,6 +2281,54 @@ Browser_Album_List_Set_Row_Appearance (GtkTreeIter *iter)
     }
 }
 
+/*
+ * In the <All Albums> Row is a List attached which has to be freed.
+ */
+
+void Browser_Album_List_Clear( void )
+{
+	GList *lstPtr;
+	GtkTreeIter selectedIter;
+	gboolean valid, allAlbumsRow;
+    GtkTreeSelection *selection;
+
+    valid = gtk_tree_model_get_iter_first( GTK_TREE_MODEL(albumListModel), &selectedIter );
+    while ( valid )
+    {
+    	allAlbumsRow = FALSE;
+
+    	gtk_tree_model_get( GTK_TREE_MODEL(albumListModel), &selectedIter,
+    			ALBUM_ETFILE_LIST_POINTER, &lstPtr,
+    			ALBUM_ALL_ALBUMS_ROW, &allAlbumsRow,
+    			-1);
+
+    	if ( allAlbumsRow && lstPtr )
+    	{
+    		g_list_free( lstPtr );
+    	}
+
+    	valid = gtk_tree_model_iter_next( GTK_TREE_MODEL(albumListModel), &selectedIter );
+    }
+
+    /*
+     * Empty Model, Disable Browser_Album_List_Row_Selected() during clear because it is
+     * called and crashes
+     */
+
+    selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(BrowserAlbumList));
+
+    g_signal_handlers_block_by_func(G_OBJECT(selection),G_CALLBACK(Browser_Album_List_Row_Selected),NULL);
+
+    gtk_list_store_clear( albumListModel );
+
+    g_signal_handlers_unblock_by_func(G_OBJECT(selection),G_CALLBACK(Browser_Album_List_Row_Selected),NULL);
+}
+
+void Browser_Artist_List_Clear( void )
+{
+    gtk_list_store_clear( artistListModel );
+}
+
 void Browser_Display_Tree_Or_Artist_Album_List (void)
 {
     ET_File *etfile = ETCore->ETFileDisplayed; // ETFile to display again after changing browser view
@@ -3246,7 +3289,7 @@ GtkWidget *Create_Browser_Items (GtkWidget *parent)
                                          GDK_TYPE_COLOR);
 
     BrowserArtistList = gtk_tree_view_new_with_model(GTK_TREE_MODEL(artistListModel));
-    g_object_unref (artistListModel);
+    //g_object_unref (artistListModel);
     gtk_tree_view_set_headers_visible(GTK_TREE_VIEW(BrowserArtistList), TRUE);
     gtk_tree_selection_set_mode(gtk_tree_view_get_selection(GTK_TREE_VIEW(BrowserArtistList)),GTK_SELECTION_SINGLE);
 
@@ -3324,10 +3367,11 @@ GtkWidget *Create_Browser_Items (GtkWidget *parent)
                                          G_TYPE_POINTER,
                                          PANGO_TYPE_STYLE,
                                          G_TYPE_INT,
-                                         GDK_TYPE_COLOR);
+                                         GDK_TYPE_COLOR,
+                                         G_TYPE_BOOLEAN);
 
     BrowserAlbumList = gtk_tree_view_new_with_model(GTK_TREE_MODEL(albumListModel));
-    g_object_unref (albumListModel);
+    //g_object_unref (albumListModel);
     gtk_tree_view_set_headers_visible(GTK_TREE_VIEW(BrowserAlbumList), TRUE);
     gtk_tree_view_set_reorderable(GTK_TREE_VIEW(BrowserAlbumList), FALSE);
     gtk_tree_selection_set_mode(gtk_tree_view_get_selection(GTK_TREE_VIEW(BrowserAlbumList)), GTK_SELECTION_SINGLE);
@@ -3415,7 +3459,7 @@ GtkWidget *Create_Browser_Items (GtkWidget *parent)
                                         GDK_TYPE_COLOR); /* Row foreground. */
 
     BrowserList = gtk_tree_view_new_with_model(GTK_TREE_MODEL(fileListModel));
-    g_object_unref (fileListModel);
+    //g_object_unref (fileListModel);
     gtk_container_add(GTK_CONTAINER(ScrollWindowFileList), BrowserList);
     gtk_tree_view_set_headers_visible(GTK_TREE_VIEW(BrowserList), TRUE);
     gtk_tree_view_set_rules_hint(GTK_TREE_VIEW(BrowserList), FALSE);
diff --git a/src/browser.h b/src/browser.h
index 5965379..9121767 100644
--- a/src/browser.h
+++ b/src/browser.h
@@ -121,6 +121,7 @@ enum
     ALBUM_FONT_STYLE,
     ALBUM_FONT_WEIGHT,
     ALBUM_ROW_FOREGROUND,
+    ALBUM_ALL_ALBUMS_ROW,			// Used to identify the correct row to free the inserted list
     ALBUM_COLUMN_COUNT
 };
 
@@ -189,4 +190,7 @@ void         Browser_Open_Run_Program_List_Window (void);
 GtkTreeViewColumn *get_column_for_column_id (gint column_id);
 GtkSortType get_sort_order_for_column_id (gint column_id);
 
+void Browser_Album_List_Clear( void );
+void Browser_Artist_List_Clear( void );
+
 #endif /* __BROWSER_H__ */
diff --git a/src/et_core.c b/src/et_core.c
index e699c39..c5254ae 100644
--- a/src/et_core.c
+++ b/src/et_core.c
@@ -285,7 +285,7 @@ void ET_Core_Initialize (void)
 {
     ETCore->ETFileList                        = NULL;
     ETCore->ETFileDisplayedList               = NULL;
-    ETCore->ETFileDisplayedListPtr            = NULL;
+/*    ETCore->ETFileDisplayedListPtr            = NULL; */
     ETCore->ETFileDisplayedList_Length        = 0;
     ETCore->ETFileDisplayedList_TotalSize     = 0;
     ETCore->ETFileDisplayedList_TotalDuration = 0;
@@ -2389,19 +2389,30 @@ ET_Free_Artist_Album_File_List (void)
     g_return_val_if_fail (ETCore != NULL
                           && ETCore->ETArtistAlbumFileList != NULL, FALSE);
 
+    /*
+     * Pointers are stored inside the artist/album Models. So free them first
+     */
+
+    Browser_Artist_List_Clear();
+    Browser_Album_List_Clear();
+
+    /*
+     * Now clear the list
+     */
+
     for (l = ETCore->ETArtistAlbumFileList; l != NULL; l = g_list_next (l))
     {
-        GList *m;
+    	GList *m;
 
-	for (m = (GList *)l->data; m != NULL; m = g_list_next (m))
-        {
-            GList *n = (GList *)m->data;
-            if (n)
-                g_list_free (n);
-        }
+    	for (m = (GList *)l->data; m != NULL; m = g_list_next (m))
+    	{
+    		GList *n = (GList *)m->data;
+    		if (n)
+    			g_list_free (n);
+    	}
 
-        if (l->data) /* Free AlbumList list. */
-            g_list_free ((GList *)l->data);
+    	if (l->data) /* Free AlbumList list. */
+    		g_list_free ((GList *)l->data);
     }
 
     if (ETCore->ETArtistAlbumFileList)
diff --git a/src/et_core.h b/src/et_core.h
index 5476550..c76527d 100644
--- a/src/et_core.h
+++ b/src/et_core.h
@@ -285,7 +285,7 @@ struct _ET_Core
 
     // Displayed list (part of the main list of files displayed in BrowserList) (used when displaying by Artist & Album) 
     GList *ETFileDisplayedList;                 // List of files displayed (List of ET_File from ETFileList / ATArtistAlbumFileList) | !! May not point to the first item!!
-    GList *ETFileDisplayedListPtr;              // Pointer to the current item of ETFileDisplayedList used with ET_Displayed_File_List_First, ET_Displayed_File_List_Previous
+//    GList *ETFileDisplayedListPtr;              // Pointer to the current item of ETFileDisplayedList used with ET_Displayed_File_List_First, ET_Displayed_File_List_Previous
     guint  ETFileDisplayedList_Length;          // Contains the length of the displayed list
     gfloat ETFileDisplayedList_TotalSize;       // Total of the size of files in displayed list (in bytes)
     gulong ETFileDisplayedList_TotalDuration;   // Total of duration of files in displayed list (in seconds)
-- 
1.8.4.5