GNOME Bugzilla – Bug 617769
Bring back support for file caching when gvfs-fuse-daemon is not available
Last modified: 2010-08-03 00:47:40 UTC
Created attachment 160350 [details] [review] proposed patch The commit http://git.gnome.org/browse/file-roller/commit/?id=73fa5d77a60861cd35c6f3425d27c88061af081c introduced trouble when user was trying to work with archives on remote GIO mounts and not having the fuse daemon running. This led to segfaults and error messages. This patch basically reverts that commit and introduces few new fixes to make things working: - fixed crash on extract where cancellable was freed and reused afterwards - about half of the gio-utils.c functions have been ported to GIO, removing complicated string operations for path handling (extracting parent directory, concatenating paths, etc.) In addition, I consider the following bugs to be fixed with this patch: Bug 588813 - Creating an archive from a ssh mountpoint doesn't always work Bug 594624 - Extracting into a sftp folder does not work Bug 565606 - Creating archive on remote location (samba share) does not work Bug 578892 - file-roller only creates empty folder when extracting to network-share Possibly also Bug 582471 - file-roller crashed with SIGSEGV in IA__g_object_ref() Patch is against git master (2.31.0 at the date of writing).
Review of attachment 160350 [details] [review]: The patch looks good to me, the only issue is some memory leaks due to g_file_get_uri(). ::: src/gio-utils.c @@ +590,3 @@ char *dir; + dir = g_strdup (g_file_get_uri (gfl->base_dir)); there is a memory leak here, it should be: dir = g_file_get_uri (gfl->base_dir); @@ +602,3 @@ g_hash_table_insert (h_dirs, (char*)scan->data, GINT_TO_POINTER (1)); + gfl->dirs = g_list_concat (gfl->dirs, get_dir_list_from_file_list (h_dirs, g_file_get_uri (gfl->base_dir), gfl->files, FALSE)); memory leak caused by g_file_get_uri @@ +606,2 @@ if (filter_empty (gfl->include_filter)) + gfl->dirs = g_list_concat (gfl->dirs, get_dir_list_from_file_list (h_dirs, g_file_get_uri (gfl->base_dir), gfl->dirs, TRUE)); same here @@ +782,2 @@ + g_directory_list_all_async (g_file_get_uri (directory_file), + g_file_get_uri (gfl->base_dir), another memory leak
Created attachment 160893 [details] [review] proposed patch Fixed patch, thanks for spotting the leaks.
Patch applied to master now, thank you. However I think this solution is suboptimal, the issue should be fixed in gio.
Hi, The patch seems to have regression on pathname handling: https://bugzilla.redhat.com/show_bug.cgi?id=610338 In gio-utils.c:get_items_for_current_dir(): directory_name = file_name_from_path ((char*) gfl->current_dir->data); Since now gfl->current_dir->data holds a URI not a local pathname, file_name_from_path() cannot be used directly. For example, if gfl->current_dir->data is "file:///home/ueno/test%20test", it should be first decoded to "/home/ueno/test test". I'm attaching a patch.
Created attachment 166946 [details] [review] patch to avoid double encoding of pathnames
Created attachment 166948 [details] [review] patch to avoid double encoding of pathnames fixed memleak
Created attachment 166949 [details] [review] patch to avoid double encoding of pathnames
I fixed it using g_file_get_basename, it's better to not use g_file_get_path because it can return NULL if the remote uri is not mounted.
Thanks. I confirmed that the commit 23c4cf9e fixed the bug.