[easytag/wip/glist: 2/8] Fix many memory leaks when freeing lists



commit 4bbb44f4eba67136753830fa780792515b5d5477
Author: David King <amigadave amigadave com>
Date:   Wed May 22 19:04:15 2013 +0100

    Fix many memory leaks when freeing lists
    
    Take care to iterate over the whole GList when freeing the contained
    elements as well as the list, and fix many memory leaks in the process.

 src/easytag.c |   39 +++++++++++-----------
 src/et_core.c |  101 ++++++++++++++++++++------------------------------------
 src/log.c     |   19 +++++------
 src/scan.c    |   40 +++++++++++-----------
 4 files changed, 85 insertions(+), 114 deletions(-)
---
diff --git a/src/easytag.c b/src/easytag.c
index 9945605..41e12f3 100644
--- a/src/easytag.c
+++ b/src/easytag.c
@@ -1976,6 +1976,7 @@ void Action_Scan_Selected_Files (void)
     gchar progress_bar_text[30];
     double fraction;
     GList *selfilelist = NULL;
+    GList *l;
     ET_File *etfile;
     GtkTreeSelection *selection;
 
@@ -2007,9 +2008,10 @@ void Action_Scan_Selected_Files (void)
 
     selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(BrowserList));
     selfilelist = gtk_tree_selection_get_selected_rows(selection, NULL);
-    while (selfilelist)
+
+    for (l = selfilelist; l != NULL; l = g_list_next (l))
     {
-        etfile = Browser_List_Get_ETFile_From_Path(selfilelist->data);
+        etfile = Browser_List_Get_ETFile_From_Path (l->data);
 
         // Run the current scanner
         Scan_Select_Mode_And_Run_Scanner(etfile);
@@ -2022,13 +2024,9 @@ void Action_Scan_Selected_Files (void)
         /* Needed to refresh status bar */
         while (gtk_events_pending())
             gtk_main_iteration();
-
-        if (!selfilelist->next) break;
-        selfilelist = g_list_next(selfilelist);
     }
 
-    g_list_foreach(selfilelist, (GFunc) gtk_tree_path_free, NULL);
-    g_list_free(selfilelist);
+    g_list_free_full (selfilelist, (GDestroyNotify)gtk_tree_path_free);
 
     // Refresh the whole list (faster than file by file) to show changes
     Browser_List_Refresh_Whole_List();
@@ -3247,6 +3245,7 @@ gboolean Read_Directory (gchar *path_real)
     guint  nbrfile = 0;
     double fraction;
     GList *FileList = NULL;
+    GList *l;
     gint   progress_bar_index = 0;
     GtkAction *uiaction;
     GtkWidget *artist_radio;
@@ -3337,9 +3336,10 @@ gboolean Read_Directory (gchar *path_real)
     gtk_progress_bar_set_text(GTK_PROGRESS_BAR(ProgressBar), progress_bar_text);
 
     // Load the supported files (Extension recognized)
-    while (FileList)
+    for (l = FileList; l != NULL && !Main_Stop_Button_Pressed;
+         l = g_list_next (l))
     {
-        gchar *filename_real = FileList->data; // Contains real filenames
+        gchar *filename_real = l->data; /* Contains real filenames. */
         gchar *filename_utf8 = filename_to_display(filename_real);
 
         msg = g_strdup_printf(_("File: '%s'"),filename_utf8);
@@ -3357,11 +3357,10 @@ gboolean Read_Directory (gchar *path_real)
         gtk_progress_bar_set_text(GTK_PROGRESS_BAR(ProgressBar), progress_bar_text);
         while (gtk_events_pending())
             gtk_main_iteration();
-
-        if (!FileList->next || Main_Stop_Button_Pressed) break;
-        FileList = FileList->next;
     }
-    if (FileList) g_list_free(FileList);
+
+    /* Just free the list, not the data. */
+    g_list_free (FileList);
     gtk_progress_bar_set_text(GTK_PROGRESS_BAR(ProgressBar), "");
 
     /* Close window to quit recursion */
@@ -3773,20 +3772,22 @@ void Update_Command_Buttons_Sensivity (void)
         /* Check if one of the selected files has undo or redo data */
         if (BrowserList)
         {
+            GList *l;
+
             selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(BrowserList));
             selfilelist = gtk_tree_selection_get_selected_rows(selection, NULL);
-            while (selfilelist)
+
+            for (l = selfilelist; l != NULL; l = g_list_next (l))
             {
-                etfile = Browser_List_Get_ETFile_From_Path(selfilelist->data);
+                etfile = Browser_List_Get_ETFile_From_Path (l->data);
                 has_undo    |= ET_File_Data_Has_Undo_Data(etfile);
                 has_redo    |= ET_File_Data_Has_Redo_Data(etfile);
                 //has_to_save |= ET_Check_If_File_Is_Saved(etfile);
-                if ((has_undo && has_redo /*&& has_to_save*/) || !selfilelist->next) // Useless to check the 
other files
+                if ((has_undo && has_redo /*&& has_to_save*/) || !l->next) // Useless to check the other 
files
                     break;
-                selfilelist = g_list_next(selfilelist);
             }
-            g_list_foreach(selfilelist, (GFunc) gtk_tree_path_free, NULL);
-            g_list_free(selfilelist);
+
+            g_list_free_full (selfilelist, (GDestroyNotify)gtk_tree_path_free);
         }
 
         /* Enable undo commands if there are undo data */
diff --git a/src/et_core.c b/src/et_core.c
index 0d4bbe0..709ed9c 100644
--- a/src/et_core.c
+++ b/src/et_core.c
@@ -2068,7 +2068,7 @@ gboolean ET_Set_Displayed_File_List (GList *ETFileList)
  */
 gboolean ET_Free_File_List (void)
 {
-    g_return_val_if_fail (ETCore != NULL || ETCore->ETFileList != NULL, FALSE);
+    g_return_val_if_fail (ETCore != NULL && ETCore->ETFileList != NULL, FALSE);
 
     g_list_free_full (ETCore->ETFileList,
                       (GDestroyNotify)ET_Free_File_List_Item);
@@ -2109,8 +2109,8 @@ gboolean ET_Free_File_List_Item (ET_File *ETFile)
         }
         g_free(ETFile->ETFileExtension);
         g_free(ETFile);
-        ETFile = NULL;
     }
+
     return TRUE;
 }
 
@@ -2120,21 +2120,12 @@ gboolean ET_Free_File_List_Item (ET_File *ETFile)
  */
 gboolean ET_Free_File_Name_List (GList *FileNameList)
 {
-    GList *list;
-
     g_return_val_if_fail (FileNameList != NULL, FALSE);
 
-    list = g_list_last(FileNameList);
-    while (list)
-    {
-        if ( (File_Name *)list->data )
-            ET_Free_File_Name_Item( (File_Name *)list->data );
+    FileNameList = g_list_first (FileNameList);
+
+    g_list_free_full (FileNameList, (GDestroyNotify)ET_Free_File_Name_Item);
 
-        if (!list->prev) break;
-        list = list->prev;
-    }
-    g_list_free(list);
-    FileNameList = NULL;
     return TRUE;
 }
 
@@ -2150,7 +2141,7 @@ gboolean ET_Free_File_Name_Item (File_Name *FileName)
     g_free(FileName->value_utf8);
     g_free(FileName->value_ck);
     g_free(FileName);
-    FileName = NULL;
+
     return TRUE;
 }
 
@@ -2161,21 +2152,20 @@ gboolean ET_Free_File_Name_Item (File_Name *FileName)
 static gboolean
 ET_Free_File_Tag_List (GList *FileTagList)
 {
-    GList *list;
+    GList *l;
 
     g_return_val_if_fail (FileTagList != NULL, FALSE);
 
-    list = g_list_last(FileTagList);
-    while (list)
-    {
-        if ( (File_Tag *)list->data )
-            ET_Free_File_Tag_Item( (File_Tag *)list->data );
+    FileTagList = g_list_first (FileTagList);
 
-        if (!list->prev) break;
-        list = list->prev;
+    for (l = FileTagList; l != NULL; l = g_list_next (l))
+    {
+        if ((File_Tag *)l->data)
+            ET_Free_File_Tag_Item ((File_Tag *)l->data);
     }
-    g_list_free(list);
-    FileTagList = NULL;
+
+    g_list_free (FileTagList);
+
     return TRUE;
 }
 
@@ -2186,16 +2176,7 @@ ET_Free_File_Tag_List (GList *FileTagList)
 static gboolean
 ET_Free_File_Tag_Item_Other_Field (File_Tag *FileTag)
 {
-    GList *other = FileTag->other;
-
-    while (other)
-    {
-        g_free(other->data);
-
-        if (!other->next) break;
-        other = other->next;
-    }
-    g_list_free(FileTag->other);
+    g_list_free_full (FileTag->other, g_free);
 
     return TRUE;
 }
@@ -2256,22 +2237,15 @@ ET_Free_File_Info_Item (ET_File_Info *ETFileInfo)
 static gboolean
 ET_Free_History_File_List (void)
 {
-    GList *list;
-
-    g_return_val_if_fail (ETCore != NULL || ETCore->ETHistoryFileList != NULL,
+    g_return_val_if_fail (ETCore != NULL && ETCore->ETHistoryFileList != NULL,
                           FALSE);
 
-    ETCore->ETHistoryFileList = g_list_first(ETCore->ETHistoryFileList);
-    list = ETCore->ETHistoryFileList;
-    while (list)
-    {
-        g_free( (ET_History_File *)list->data );
+    ETCore->ETHistoryFileList = g_list_first (ETCore->ETHistoryFileList);
+
+    g_list_free_full (ETCore->ETHistoryFileList, g_free);
 
-        if (!list->next) break;
-        list = list->next;
-    }
-    g_list_free(ETCore->ETHistoryFileList);
     ETCore->ETHistoryFileList = NULL;
+
     return TRUE;
 }
 
@@ -2281,8 +2255,8 @@ ET_Free_History_File_List (void)
 static gboolean
 ET_Free_Displayed_File_List (void)
 {
-    g_return_val_if_fail (ETCore != NULL ||
-                          ETCore->ETFileDisplayedList != NULL, FALSE);
+    g_return_val_if_fail (ETCore != NULL
+                          && ETCore->ETFileDisplayedList != NULL, FALSE);
 
     ETCore->ETFileDisplayedList = g_list_first(ETCore->ETFileDisplayedList);
     ETCore->ETFileDisplayedList = NULL;
@@ -2297,29 +2271,26 @@ ET_Free_Displayed_File_List (void)
 static gboolean
 ET_Free_Artist_Album_File_List (void)
 {
-    GList *ArtistList;
-    GList *AlbumList;
-    GList *etfilelist;
+    GList *l;
 
-    g_return_val_if_fail (ETCore != NULL ||
-                          ETCore->ETArtistAlbumFileList != NULL, FALSE);
+    g_return_val_if_fail (ETCore != NULL
+                          && ETCore->ETArtistAlbumFileList != NULL, FALSE);
 
-    ArtistList = ETCore->ETArtistAlbumFileList;
-    while (ArtistList)
+    for (l = ETCore->ETArtistAlbumFileList; l != NULL; l = g_list_next (l))
     {
-        AlbumList = (GList *)ArtistList->data;
-        while (AlbumList)
+        GList *m;
+
+       for (m = (GList *)l->data; m != NULL; m = g_list_next (m))
         {
-            etfilelist = (GList *)AlbumList->data;
-            if (etfilelist)
-                g_list_free(etfilelist);
-            AlbumList = AlbumList->next;
+            GList *n = (GList *)m->data;
+            if (n)
+                g_list_free (n);
         }
-        if (ArtistList->data) // Free AlbumList list
-            g_list_free((GList *)ArtistList->data);
 
-        ArtistList = ArtistList->next;
+        if (l->data) /* Free AlbumList list. */
+            g_list_free ((GList *)l->data);
     }
+
     if (ETCore->ETArtistAlbumFileList)
         g_list_free(ETCore->ETArtistAlbumFileList);
 
diff --git a/src/log.c b/src/log.c
index de2943d..cfdc9f8 100644
--- a/src/log.c
+++ b/src/log.c
@@ -413,21 +413,20 @@ Log_Print_Tmp_List (void)
     // Free the list...
     if (LogPrintTmpList)
     {
-        LogPrintTmpList = g_list_first(LogPrintTmpList);
-        while (LogPrintTmpList)
-        {
-            g_free(((Log_Data *)LogPrintTmpList->data)->string);
-            g_free(((Log_Data *)LogPrintTmpList->data)->time);
-            g_free(((Log_Data *)LogPrintTmpList->data));
+        GList *l;
+
+        LogPrintTmpList = g_list_first (LogPrintTmpList);
 
-            if (!LogPrintTmpList->next) break;
-            LogPrintTmpList = LogPrintTmpList->next;
+        for (l = LogPrintTmpList; l != NULL; l = g_list_next (l))
+        {
+            g_free (((Log_Data *)l->data)->string);
+            g_free (((Log_Data *)l->data)->time);
+            g_free (((Log_Data *)l->data));
         }
 
-        g_list_free(LogPrintTmpList);
+        g_list_free (LogPrintTmpList);
         LogPrintTmpList = NULL;
     }
-
 }
 
 
diff --git a/src/scan.c b/src/scan.c
index e6f6f8e..a98503b 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -625,20 +625,20 @@ void Scan_Fill_Tag_Generate_Preview (void)
 static void
 Scan_Free_File_Fill_Tag_List (GList *list)
 {
-    // Free the list
-    list = g_list_first(list);
-    while (list)
+    GList *l;
+
+    list = g_list_first (list);
+
+    for (l = list; l != NULL; l = g_list_next (l))
     {
-        if (list->data)
+        if (l->data)
         {
-            g_free(((Scan_Mask_Item *)list->data)->string);
-            g_free( (Scan_Mask_Item *)list->data );
+            g_free (((Scan_Mask_Item *)l->data)->string);
+            g_free ((Scan_Mask_Item *)l->data);
         }
-        if (!list->next) break;
-        list = list->next;
     }
-    g_list_free(list);
-    list = NULL;
+
+    g_list_free (list);
 }
 
 
@@ -985,20 +985,20 @@ void Scan_Rename_File_Generate_Preview (void)
 static void
 Scan_Free_File_Rename_List (GList *list)
 {
-    // Free the list
-    list = g_list_last(list);
-    while (list)
+    GList *l;
+
+    list = g_list_first (list);
+
+    for (l = list; l != NULL; l = g_list_next (l))
     {
-        if (list->data)
+        if (l->data)
         {
-            g_free(((File_Mask_Item *)list->data)->string);
-            g_free( (File_Mask_Item *)list->data );
+            g_free (((File_Mask_Item *)l->data)->string);
+            g_free ((File_Mask_Item *)l->data);
         }
-        if (!list->prev) break;
-        list = list->prev;
     }
-    g_list_free(list);
-    list = NULL;
+
+    g_list_free (list);
 }
 
 /*


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]