[nautilus/wip/csoriano/cleanups: 1/4] file: refactor eel-partition for better ownership management
- From: Carlos Soriano Sánchez <csoriano src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [nautilus/wip/csoriano/cleanups: 1/4] file: refactor eel-partition for better ownership management
- Date: Wed, 9 Dec 2015 07:05:48 +0000 (UTC)
commit 5b05a987ef818fe55d71bfc3397adb25a472449c
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,
- ©_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]
