[nautilus] file: refactor eel-partition for better ownership management



commit 864c815479a25a2b99ca354ff20edf5804d46070
Author: Carlos Soriano <csoriano gnome org>
Date:   Tue Dec 8 14:28:35 2015 +0100

    file: refactor eel-partition for better ownership management
    
    Instead of a generic function to filter GLists, implement a simpler
    and clearer filter function for file lists, since it was the only
    use of that function.
    In this way the ownership of files and directories are clearer
    since it always returns a new allocated nautilus file list, and also
    it always uses nautilus_file_ref instead of the generic g_object_ref
    to match what we do everywhere else in nautilus, so it's not confusing
    when breaking at nautilus_file_ref/unref for ref counting debugging.
    
    This change fixes multiple leaks on nautilus files catched by valgrind.

 eel/eel-glib-extensions.c                |  124 ------------------------------
 eel/eel-glib-extensions.h                |    5 -
 eel/eel-lib-self-check-functions.h       |    1 -
 libnautilus-private/nautilus-directory.c |   14 +--
 libnautilus-private/nautilus-file.c      |   47 +++++++++--
 libnautilus-private/nautilus-file.h      |    7 ++
 src/nautilus-files-view.c                |   29 ++++---
 7 files changed, 66 insertions(+), 161 deletions(-)
---
diff --git a/eel/eel-glib-extensions.c b/eel/eel-glib-extensions.c
index 1f1dc5f..709c37a 100644
--- a/eel/eel-glib-extensions.c
+++ b/eel/eel-glib-extensions.c
@@ -92,67 +92,6 @@ eel_g_lists_sort_and_check_for_intersection (GList **list_1,
        return FALSE;
 }
 
-
-/**
- * eel_g_list_partition
- * 
- * Parition a list into two parts depending on whether the data
- * elements satisfy a provided predicate. Order is preserved in both
- * of the resulting lists, and the original list is consumed. A list
- * of the items that satisfy the predicate is returned, and the list
- * of items not satisfying the predicate is returned via the failed
- * out argument.
- * 
- * @list: List to partition.
- * @predicate: Function to call on each element.
- * @user_data: Data to pass to function.  
- * @failed: The GList * variable pointed to by this argument will be
- * set to the list of elements for which the predicate returned
- * false. */
-
-GList *
-eel_g_list_partition (GList *list,
-                          EelPredicateFunction  predicate,
-                          gpointer user_data,
-                          GList **failed)
-{
-       GList *predicate_true;
-       GList *predicate_false;
-       GList *reverse;
-       GList *p;
-       GList *next;
-
-       predicate_true = NULL;
-       predicate_false = NULL;
-
-       reverse = g_list_reverse (list);
-
-       for (p = reverse; p != NULL; p = next) {
-               next = p->next;
-               
-               if (next != NULL) {
-                       next->prev = NULL;
-               }
-
-               if (predicate (p->data, user_data)) {
-                       p->next = predicate_true;
-                       if (predicate_true != NULL) {
-                               predicate_true->prev = p;
-                       }
-                       predicate_true = p;
-               } else {
-                       p->next = predicate_false;
-                       if (predicate_false != NULL) {
-                               predicate_false->prev = p;
-                       }
-                       predicate_false = p;
-               }
-       }
-
-       *failed = predicate_false;
-       return predicate_true;
-}
-
 typedef struct {
        GList *keys;
        GList *values;
@@ -197,67 +136,4 @@ eel_g_hash_table_safe_for_each (GHashTable *hash_table,
 
 #if !defined (EEL_OMIT_SELF_CHECK)
 
-static gboolean
-eel_test_str_list_equal (GList *list_a, GList *list_b)
-{
-       GList *p, *q;
-
-       for (p = list_a, q = list_b; p != NULL && q != NULL; p = p->next, q = q->next) {
-               if (g_strcmp0 (p->data, q->data) != 0) {
-                       return FALSE;
-               }
-       }
-       return p == NULL && q == NULL;
-}
-
-static gboolean
-eel_test_predicate (gpointer data,
-                   gpointer callback_data)
-{
-       return g_ascii_strcasecmp (data, callback_data) <= 0;
-}
-
-void
-eel_self_check_glib_extensions (void)
-{
-       GList *list_to_partition;
-       GList *expected_passed;
-       GList *expected_failed;
-       GList *actual_passed;
-       GList *actual_failed;
-       
-       /* eel_g_list_partition */
-
-       list_to_partition = NULL;
-       list_to_partition = g_list_append (list_to_partition, "Cadillac");
-       list_to_partition = g_list_append (list_to_partition, "Pontiac");
-       list_to_partition = g_list_append (list_to_partition, "Ford");
-       list_to_partition = g_list_append (list_to_partition, "Range Rover");
-       
-       expected_passed = NULL;
-       expected_passed = g_list_append (expected_passed, "Cadillac");
-       expected_passed = g_list_append (expected_passed, "Ford");
-       
-       expected_failed = NULL;
-       expected_failed = g_list_append (expected_failed, "Pontiac");
-       expected_failed = g_list_append (expected_failed, "Range Rover");
-       
-       actual_passed = eel_g_list_partition (list_to_partition, 
-                                             eel_test_predicate,
-                                             "m",
-                                             &actual_failed);
-       
-       EEL_CHECK_BOOLEAN_RESULT (eel_test_str_list_equal (expected_passed, actual_passed), TRUE);
-       EEL_CHECK_BOOLEAN_RESULT (eel_test_str_list_equal (expected_failed, actual_failed), TRUE);
-       
-       /* Don't free "list_to_partition", since it is consumed
-        * by eel_g_list_partition.
-        */
-       
-       g_list_free (expected_passed);
-       g_list_free (actual_passed);
-       g_list_free (expected_failed);
-       g_list_free (actual_failed);
-}
-
 #endif /* !EEL_OMIT_SELF_CHECK */
diff --git a/eel/eel-glib-extensions.h b/eel/eel-glib-extensions.h
index 2ccb862..f233024 100644
--- a/eel/eel-glib-extensions.h
+++ b/eel/eel-glib-extensions.h
@@ -39,11 +39,6 @@ typedef gboolean (* EelPredicateFunction) (gpointer data,
 /* GList functions. */
 gboolean    eel_g_lists_sort_and_check_for_intersection (GList                **list_a,
                                                         GList                **list_b);
-GList *     eel_g_list_partition                        (GList                 *list,
-                                                        EelPredicateFunction   predicate,
-                                                        gpointer               user_data,
-                                                        GList                **removed);
-
 /* GHashTable functions */
 void        eel_g_hash_table_safe_for_each              (GHashTable            *hash_table,
                                                         GHFunc                 callback,
diff --git a/eel/eel-lib-self-check-functions.h b/eel/eel-lib-self-check-functions.h
index ed9de8f..11386e8 100644
--- a/eel/eel-lib-self-check-functions.h
+++ b/eel/eel-lib-self-check-functions.h
@@ -37,7 +37,6 @@ void eel_run_lib_self_checks (void);
 */
 
 #define EEL_LIB_FOR_EACH_SELF_CHECK_FUNCTION(macro) \
-       macro (eel_self_check_glib_extensions) \
        macro (eel_self_check_string) \
 /* Add new self-check functions to the list above this line. */
 
diff --git a/libnautilus-private/nautilus-directory.c b/libnautilus-private/nautilus-directory.c
index 95f2a92..d546250 100644
--- a/libnautilus-private/nautilus-directory.c
+++ b/libnautilus-private/nautilus-directory.c
@@ -1545,13 +1545,11 @@ nautilus_directory_is_not_empty (NautilusDirectory *directory)
 }
 
 static gboolean
-is_tentative (gpointer data, gpointer callback_data)
+is_tentative (NautilusFile *file,
+              gpointer      callback_data)
 {
-       NautilusFile *file;
-
        g_assert (callback_data == NULL);
 
-       file = NAUTILUS_FILE (data);
        /* Avoid returning files with !is_added, because these
         * will later be sent with the files_added signal, and a
         * user doing get_file_list + files_added monitoring will
@@ -1570,12 +1568,10 @@ real_get_file_list (NautilusDirectory *directory)
 {
        GList *tentative_files, *non_tentative_files;
 
-       tentative_files = eel_g_list_partition
-               (g_list_copy (directory->details->file_list),
-                is_tentative, NULL, &non_tentative_files);
-       g_list_free (tentative_files);
+       tentative_files = nautilus_file_list_filter (directory->details->file_list,
+                                                     &non_tentative_files, is_tentative, NULL);
+       nautilus_file_list_free (tentative_files);
 
-       nautilus_file_list_ref (non_tentative_files);
        return non_tentative_files;
 }
 
diff --git a/libnautilus-private/nautilus-file.c b/libnautilus-private/nautilus-file.c
index 075690f..e1219f6 100644
--- a/libnautilus-private/nautilus-file.c
+++ b/libnautilus-private/nautilus-file.c
@@ -3394,13 +3394,11 @@ nautilus_file_is_in_search (NautilusFile *file)
 }
 
 static gboolean
-filter_hidden_partition_callback (gpointer data,
-                                 gpointer callback_data)
+filter_hidden_partition_callback (NautilusFile *file,
+                                  gpointer      callback_data)
 {
-       NautilusFile *file;
        FilterOptions options;
 
-       file = NAUTILUS_FILE (data);
        options = GPOINTER_TO_INT (callback_data);
 
        return nautilus_file_should_show (file,
@@ -3419,16 +3417,47 @@ nautilus_file_list_filter_hidden (GList    *files,
         * Eventually this should become a generic filtering thingy.
         */
 
-       filtered_files = nautilus_file_list_copy (files);
-       filtered_files = eel_g_list_partition (filtered_files,
-                                              filter_hidden_partition_callback,
-                                              GINT_TO_POINTER ((show_hidden ? SHOW_HIDDEN : 0)),
-                                              &removed_files);
+       filtered_files = nautilus_file_list_filter (files,
+                                                   &removed_files,
+                                                   filter_hidden_partition_callback,
+                                                   GINT_TO_POINTER ((show_hidden ? SHOW_HIDDEN : 0)));
        nautilus_file_list_free (removed_files);
 
        return filtered_files;
 }
 
+/* This functions filters a file list when its items match a certain condition
+ * in the filter function. This function preserves the ordering.
+ */
+GList *
+nautilus_file_list_filter (GList                   *files,
+                           GList                  **failed,
+                           NautilusFileFilterFunc   filter_function,
+                           gpointer                 user_data)
+{
+        GList *filtered = NULL;
+        GList *l;
+        GList *last;
+        GList *reversed;
+
+        *failed = NULL;
+        /* Avoid using g_list_append since it's O(n) */
+        reversed = g_list_copy (files);
+        reversed = g_list_reverse (reversed);
+        last = g_list_last (reversed);
+        for (l = last; l != NULL; l = l->prev) {
+                if (filter_function (l->data, user_data)) {
+                        filtered = g_list_prepend (filtered, nautilus_file_ref (l->data));
+                } else {
+                        *failed = g_list_prepend (*failed, nautilus_file_ref (l->data));
+                }
+        }
+
+        g_list_free (reversed);
+
+        return filtered;
+}
+
 char *
 nautilus_file_get_metadata (NautilusFile *file,
                            const char *key,
diff --git a/libnautilus-private/nautilus-file.h b/libnautilus-private/nautilus-file.h
index 85a24d9..6eb8057 100644
--- a/libnautilus-private/nautilus-file.h
+++ b/libnautilus-private/nautilus-file.h
@@ -92,6 +92,8 @@ typedef enum {
 
 typedef void (*NautilusFileCallback)          (NautilusFile  *file,
                                               gpointer       callback_data);
+typedef gboolean (*NautilusFileFilterFunc)    (NautilusFile  *file,
+                                               gpointer       callback_data);
 typedef void (*NautilusFileListCallback)      (GList         *file_list,
                                               gpointer       callback_data);
 typedef void (*NautilusFileOperationCallback) (NautilusFile  *file,
@@ -452,6 +454,11 @@ void                    nautilus_file_list_call_when_ready              (GList
                                                                         gpointer                        
callback_data);
 void                    nautilus_file_list_cancel_call_when_ready       (NautilusFileListHandle         
*handle);
 
+GList *                 nautilus_file_list_filter                       (GList                          
*files,
+                                                                         GList                         
**failed,
+                                                                         NautilusFileFilterFunc          
filter_function,
+                                                                         gpointer                        
user_data);
+
 /* Debugging */
 void                    nautilus_file_dump                              (NautilusFile                   
*file);
 
diff --git a/src/nautilus-files-view.c b/src/nautilus-files-view.c
index a7caa72..572c9de 100644
--- a/src/nautilus-files-view.c
+++ b/src/nautilus-files-view.c
@@ -361,8 +361,8 @@ check_empty_states (NautilusFilesView *view)
                                 gtk_widget_show (view->details->folder_is_empty_widget);
                         }
                 }
-                nautilus_file_list_unref (filtered);
-                nautilus_file_list_unref (files);
+                nautilus_file_list_free (filtered);
+                nautilus_file_list_free (files);
         }
 }
 
@@ -3281,13 +3281,13 @@ pre_copy_move (NautilusFilesView *directory_view)
  * and (as a side effect) remove them from the debuting uri hash table.
  */
 static gboolean
-copy_move_done_partition_func (gpointer data,
-                               gpointer callback_data)
+copy_move_done_partition_func (NautilusFile *file,
+                               gpointer      callback_data)
 {
          GFile *location;
          gboolean result;
 
-        location = nautilus_file_get_location (NAUTILUS_FILE (data));
+        location = nautilus_file_get_location (file);
         result = g_hash_table_remove ((GHashTable *) callback_data, location);
         g_object_unref (location);
 
@@ -3329,6 +3329,7 @@ copy_move_done_callback (GHashTable *debuting_files,
         NautilusFilesView  *directory_view;
         CopyMoveDoneData *copy_move_done_data;
         DebutingFilesData  *debuting_files_data;
+        GList *failed_files;
 
         copy_move_done_data = (CopyMoveDoneData *) data;
         directory_view = copy_move_done_data->directory_view;
@@ -3338,11 +3339,12 @@ copy_move_done_callback (GHashTable *debuting_files,
 
                 debuting_files_data = g_new (DebutingFilesData, 1);
                 debuting_files_data->debuting_files = g_hash_table_ref (debuting_files);
-                debuting_files_data->added_files = eel_g_list_partition
-                        (copy_move_done_data->added_files,
-                         copy_move_done_partition_func,
-                         debuting_files,
-                         &copy_move_done_data->added_files);
+                debuting_files_data->added_files = nautilus_file_list_filter 
(copy_move_done_data->added_files,
+                                                                              &failed_files,
+                                                                              copy_move_done_partition_func,
+                                                                              debuting_files);
+                nautilus_file_list_free (copy_move_done_data->added_files);
+                copy_move_done_data->added_files = failed_files;
 
                 /* We're passed the same data used by pre_copy_move_add_file_callback, so disconnecting
                  * it will free data. We've already siphoned off the added_files we need, and stashed the
@@ -4778,6 +4780,7 @@ update_directory_in_scripts_menu (NautilusFilesView *view,
         }
 
         nautilus_file_list_free (file_list);
+        nautilus_file_list_free (filtered);
 
         if (!any_scripts) {
                 g_object_unref (menu);
@@ -4957,11 +4960,11 @@ update_directory_in_templates_menu (NautilusFilesView *view,
         templates_directory_uri = nautilus_get_templates_directory_uri ();
         menu = g_menu_new ();
 
-        file_list = nautilus_file_list_sort_by_display_name (filtered);
+        filtered = nautilus_file_list_sort_by_display_name (filtered);
 
         num = 0;
         any_templates = FALSE;
-        for (node = file_list; num < TEMPLATE_LIMIT && node != NULL; node = node->next, num++) {
+        for (node = filtered; num < TEMPLATE_LIMIT && node != NULL; node = node->next, num++) {
                 file = node->data;
                 if (nautilus_file_is_directory (file)) {
                         uri = nautilus_file_get_uri (file);
@@ -4989,7 +4992,7 @@ update_directory_in_templates_menu (NautilusFilesView *view,
                 }
         }
 
-        nautilus_file_list_free (file_list);
+        nautilus_file_list_free (filtered);
         g_free (templates_directory_uri);
 
         if (!any_templates) {


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