Re: [Geeqie-devel] [PATCH v2 1/1] Fix crash on drag and drop from xfe

2016-04-26 Thread Colin Clark 
On 19/04/16 01:50, Jonathan Boeing wrote:
> On Mon, 18 Apr 2016 17:39:23 -0700
> Jonathan Boeing  wrote:
>
>> diff --git a/src/ui_bookmark.c b/src/ui_bookmark.c
>> index 842b947..18f7311 100644
>> --- a/src/ui_bookmark.c
>> +++ b/src/ui_bookmark.c
>> @@ -707,13 +707,19 @@ static void bookmark_dnd_get_data(GtkWidget
>> *widget, {
>>  BookMarkData *bm = data;
>>  GList *list = NULL;
>> +GList *errors = NULL;
>>  GList *work;
>>  gchar **uris;
>>   
>>  if (!bm->editable) return;
>>   
>>  uris = gtk_selection_data_get_uris(selection_data);
>> -list = uri_filelist_from_uris(uris);
>> +list = uri_filelist_from_uris(uris, );
>> +if(errors)
>> +{
>> +warning_dialog_dnd_uri_error(errors);
>> +string_list_free(errors);
>> +}
>>  g_strfreev(uris);
>>   
>>  work = list;
> Would you rather have bookmark_dnd_get_data() call
> uri_filelist_from_gtk_selection_data()? It looks to me like it could
> have been calling it already, regardless of my patch.
>
>> +void warning_dialog_dnd_uri_error(GList *uri_error_list)
>> +{
>> +GList *work = uri_error_list;
>> +guint count = g_list_length(work);
>> +gchar *msg = g_strdup_printf("Failed to convert %d dropped
>> item(s) to files\n", count);
>> +if(count < 10)
>> +{
>> +while (work)
>> +{
>> +gchar *prev = msg;
>> +msg = g_strdup_printf("%s\n%s", prev, (gchar
>> *)work->data);
>> +work = work->next;
>> +g_free(prev);
>> +}
>> +}
>> +warning_dialog(_("Drag and Drop failed"), msg,
>> GTK_STOCK_DIALOG_WARNING, NULL);
>> +g_free(msg);
>> +}
> I'm not real happy about that strdup loop, but warning_dialog() didn't
> look like a good basis for a dialog with multiple strings (i.e. the list
> of failed conversions). Any suggestions for that, or is it fine as-is?
>
>
Hi Jonathon

I am not competent to decide whether or not this is a perfect solution - 
anyway, I would guess there is a fair bit of Geeqie that is sub-optimal.

It works. That's good enough for me. It's in the repo.


Thanks

Colin Clark



--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Geeqie-devel mailing list
Geeqie-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geeqie-devel

Re: [Geeqie-devel] [PATCH v2 1/1] Fix crash on drag and drop from xfe

2016-04-18 Thread Jonathan Boeing 
On Mon, 18 Apr 2016 17:39:23 -0700
Jonathan Boeing  wrote:

> diff --git a/src/ui_bookmark.c b/src/ui_bookmark.c
> index 842b947..18f7311 100644
> --- a/src/ui_bookmark.c
> +++ b/src/ui_bookmark.c
> @@ -707,13 +707,19 @@ static void bookmark_dnd_get_data(GtkWidget
> *widget, {
>   BookMarkData *bm = data;
>   GList *list = NULL;
> + GList *errors = NULL;
>   GList *work;
>   gchar **uris;
>  
>   if (!bm->editable) return;
>  
>   uris = gtk_selection_data_get_uris(selection_data);
> - list = uri_filelist_from_uris(uris);
> + list = uri_filelist_from_uris(uris, );
> + if(errors)
> + {
> + warning_dialog_dnd_uri_error(errors);
> + string_list_free(errors);
> + }
>   g_strfreev(uris);
>  
>   work = list;

Would you rather have bookmark_dnd_get_data() call
uri_filelist_from_gtk_selection_data()? It looks to me like it could
have been calling it already, regardless of my patch.

> +void warning_dialog_dnd_uri_error(GList *uri_error_list)
> +{
> + GList *work = uri_error_list;
> + guint count = g_list_length(work);
> + gchar *msg = g_strdup_printf("Failed to convert %d dropped
> item(s) to files\n", count);
> + if(count < 10)
> + {
> + while (work)
> + {
> + gchar *prev = msg;
> + msg = g_strdup_printf("%s\n%s", prev, (gchar
> *)work->data);
> + work = work->next;
> + g_free(prev);
> + }
> + }
> + warning_dialog(_("Drag and Drop failed"), msg,
> GTK_STOCK_DIALOG_WARNING, NULL);
> + g_free(msg);
> +}

I'm not real happy about that strdup loop, but warning_dialog() didn't
look like a good basis for a dialog with multiple strings (i.e. the list
of failed conversions). Any suggestions for that, or is it fine as-is?

Thanks,

Jonathan Boeing


--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Geeqie-devel mailing list
Geeqie-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geeqie-devel

[Geeqie-devel] [PATCH v2 1/1] Fix crash on drag and drop from xfe

2016-04-18 Thread Jonathan Boeing 
Dragged and dropped paths from xfe aren't escaped like paths from a gtk
app. A folder named 'test # one' for example causes g_filename_from_uri
to fail and return NULL, which then causes a SIGSEGV in path_to_utf8.

This patch fixes the crash by checking for failed conversions. If
conversion failed due to a bad URI, it will manually escape the URI and
try converting it again.
---
 src/ui_bookmark.c |  8 ++-
 src/uri_utils.c   | 66 +--
 src/uri_utils.h   |  5 +++--
 3 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/src/ui_bookmark.c b/src/ui_bookmark.c
index 842b947..18f7311 100644
--- a/src/ui_bookmark.c
+++ b/src/ui_bookmark.c
@@ -707,13 +707,19 @@ static void bookmark_dnd_get_data(GtkWidget *widget,
 {
BookMarkData *bm = data;
GList *list = NULL;
+   GList *errors = NULL;
GList *work;
gchar **uris;
 
if (!bm->editable) return;
 
uris = gtk_selection_data_get_uris(selection_data);
-   list = uri_filelist_from_uris(uris);
+   list = uri_filelist_from_uris(uris, );
+   if(errors)
+   {
+   warning_dialog_dnd_uri_error(errors);
+   string_list_free(errors);
+   }
g_strfreev(uris);
 
work = list;
diff --git a/src/uri_utils.c b/src/uri_utils.c
index a60e492..ceec756 100644
--- a/src/uri_utils.c
+++ b/src/uri_utils.c
@@ -15,6 +15,26 @@
 
 #include "filedata.h"
 #include "ui_fileops.h"
+#include "ui_utildlg.h"
+
+void warning_dialog_dnd_uri_error(GList *uri_error_list)
+{
+   GList *work = uri_error_list;
+   guint count = g_list_length(work);
+   gchar *msg = g_strdup_printf("Failed to convert %d dropped item(s) to 
files\n", count);
+   if(count < 10)
+   {
+   while (work)
+   {
+   gchar *prev = msg;
+   msg = g_strdup_printf("%s\n%s", prev, (gchar 
*)work->data);
+   work = work->next;
+   g_free(prev);
+   }
+   }
+   warning_dialog(_("Drag and Drop failed"), msg, 
GTK_STOCK_DIALOG_WARNING, NULL);
+   g_free(msg);
+}
 
 gchar **uris_from_pathlist(GList *list)
 {
@@ -62,28 +82,54 @@ gboolean 
uri_selection_data_set_uris_from_filelist(GtkSelectionData *selection_d
return ret;
 }
 
-GList *uri_pathlist_from_uris(gchar **uris)
+GList *uri_pathlist_from_uris(gchar **uris, GList **uri_error_list)
 {
GList *list = NULL;
guint i = 0;
+   GError *error = NULL;
 
while (uris[i])
{
-   gchar *local_path = g_filename_from_uri(uris[i], NULL, NULL);
+   gchar *local_path = g_filename_from_uri(uris[i], NULL, );
+   if (error)
+   {
+   DEBUG_1("g_filename_from_uri failed on uri \"%s\"", 
uris[i]);
+   DEBUG_1("   error %d: %s", error->code, error->message);
+   if (error->code == G_CONVERT_ERROR_BAD_URI)
+   {
+   GError *retry_error = NULL;
+   gchar *escaped = g_uri_escape_string(uris[i], 
":/", TRUE);
+   local_path = g_filename_from_uri(escaped, NULL, 
_error);
+   if(retry_error)
+   {
+   DEBUG_1("manually escaped uri \"%s\" 
also failed g_filename_from_uri", escaped);
+   DEBUG_1("   error %d: %s", 
retry_error->code, retry_error->message);
+   g_error_free(retry_error);
+   }
+   g_free(escaped);
+   }
+   g_error_free(error);
+   error = NULL;
+   if (!local_path)
+   {
+   *uri_error_list = 
g_list_prepend(*uri_error_list, g_strdup(uris[i]));
+   i++;
+   continue;
+   }
+   }
gchar *path = path_to_utf8(local_path);
g_free(local_path);
list = g_list_prepend(list, path);
i++;
}
 
+   *uri_error_list = g_list_reverse(*uri_error_list);
return g_list_reverse(list);
 }
 
-
-
-GList *uri_filelist_from_uris(gchar **uris)
+GList *uri_filelist_from_uris(gchar **uris, GList **uri_error_list)
 {
-   GList *path_list = uri_pathlist_from_uris(uris);
+   GList *path_list = uri_pathlist_from_uris(uris, uri_error_list);
GList *filelist = filelist_from_path_list(path_list);
string_list_free(path_list);
return filelist;
@@ -91,8 +137,14 @@ GList *uri_filelist_from_uris(gchar **uris)