[Spice-devel] [PATCH spice-gtk 4/4] gtk: handle clipboard CRLF conversion

[Marc-André Lureau]

On Sat, Aug 24, 2013 at 12:27 PM, Hans de Goede <hdegoede at redhat.com> wrote:
> Hi,
>
>
> On 08/23/2013 10:25 PM, Marc-André Lureau wrote:
>>
>> gtk+ internal text/utf8 is using LF conversion, on all platforms.
>> Even though the toolkit may only handle a single line ending type, we
>> may want to avoid the conversion for Spice use cases, gtk+ could learn a
>> new native utf8 target type, see also:
>> https://bugzilla.gnome.org/show_bug.cgi?id=706657
>>
>> In the meantime, the only thing we need to convert, is to/from crlf
>> guest (from/to lf). This is what this change is about.
>>
>> It has been tested successfully with the various guest/client OS
>> combinations.
>> ---
>>   gtk/Makefile.am         |  2 ++
>>   gtk/spice-gtk-session.c | 56
>> ++++++++++++++++++++++++++++++++++++++++++++-----
>>   2 files changed, 53 insertions(+), 5 deletions(-)
>>
>> diff --git a/gtk/Makefile.am b/gtk/Makefile.am
>> index 82aa9a3..5af6642 100644
>> --- a/gtk/Makefile.am
>> +++ b/gtk/Makefile.am
>> @@ -117,6 +117,8 @@ SPICE_GTK_LIBADD_COMMON =           \
>>
>>   SPICE_GTK_SOURCES_COMMON =            \
>>         glib-compat.h                   \
>> +       spice-util.c                    \
>> +       spice-util-priv.h               \
>>         spice-gtk-session.c             \
>>         spice-gtk-session-priv.h        \
>>         spice-widget.c                  \
>> diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
>> index 68777eb..476af95 100644
>> --- a/gtk/spice-gtk-session.c
>> +++ b/gtk/spice-gtk-session.c
>> @@ -23,6 +23,7 @@
>>   #include "spice-gtk-session.h"
>>   #include "spice-gtk-session-priv.h"
>>   #include "spice-session-priv.h"
>> +#include "spice-util-priv.h"
>>
>>   #define CLIPBOARD_LAST (VD_AGENT_CLIPBOARD_SELECTION_SECONDARY + 1)
>>
>> @@ -548,6 +549,7 @@ static void clipboard_owner_change(GtkClipboard
>> *clipboard,
>>
>>   typedef struct
>>   {
>> +    SpiceGtkSession *self;
>>       GMainLoop *loop;
>>       GtkSelectionData *selection_data;
>>       guint info;
>> @@ -555,21 +557,45 @@ typedef struct
>>   } RunInfo;
>>
>>   static void clipboard_got_from_guest(SpiceMainChannel *main, guint
>> selection,
>> -                                     guint type, guchar *data, guint
>> size,
>> +                                     guint type, const guchar *data,
>> guint size,
>>                                        gpointer user_data)
>>   {
>>       RunInfo *ri = user_data;
>> +    SpiceGtkSessionPrivate *s = ri->self->priv;
>> +    gchar *conv = NULL;
>>
>>       g_return_if_fail(selection == ri->selection);
>>
>>       SPICE_DEBUG("clipboard got data");
>>
>> -    gtk_selection_data_set(ri->selection_data,
>> -        gdk_atom_intern_static_string(atom2agent[ri->info].xatom),
>> -        8, data, size);
>> +    if (atom2agent[ri->info].vdagent == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
>> +        /* on windows, gtk+ would already convert to LF endings, but
>> +           not on unix */
>> +        if (spice_main_agent_test_capability(s->main,
>> VD_AGENT_CAP_GUEST_LINEEND_CRLF)) {
>> +            GError *err = NULL;
>> +
>> +            conv = spice_dos2unix((gchar*)data, size, &err);
>> +            if (err) {
>> +                g_warning("Failed to convert text line ending: %s",
>> err->message);
>> +                g_clear_error(&err);
>> +                goto end;
>> +            }
>> +
>> +            size = strlen(conv);
>> +        }
>> +
>> +        gtk_selection_data_set_text(ri->selection_data, conv ?:
>> (gchar*)data, size);
>
>
> Why the "conv ?: (gchar*)data" here ? Should conv not always be non NULL
> here, unless
> size = 0, in which case conv == NULL is fine ?
>
>
>
>
>> +    } else {
>> +        gtk_selection_data_set(ri->selection_data,
>> +            gdk_atom_intern_static_string(atom2agent[ri->info].xatom),
>> +            8, data, size);
>> +    }
>>
>> +end:
>>       if (g_main_loop_is_running (ri->loop))
>>           g_main_loop_quit (ri->loop);
>> +
>> +    g_free(conv);
>>   }
>>
>>   static void clipboard_agent_connected(RunInfo *ri)
>> @@ -604,6 +630,7 @@ static void clipboard_get(GtkClipboard *clipboard,
>>       ri.info = info;
>>       ri.loop = g_main_loop_new(NULL, FALSE);
>>       ri.selection = selection;
>> +    ri.self = self;
>>
>>       clipboard_handler = g_signal_connect(s->main,
>> "main-clipboard-selection",
>>
>> G_CALLBACK(clipboard_got_from_guest),
>> @@ -740,8 +767,27 @@ static void clipboard_received_cb(GtkClipboard
>> *clipboard,
>>           g_free(name);
>>       }
>>
>> +    const guchar *data = gtk_selection_data_get_data(selection_data);
>> +    gpointer conv = NULL;
>> +
>> +    /* gtk+ internal utf8 newline is always LF, even on windows */
>> +    if (type == VD_AGENT_CLIPBOARD_UTF8_TEXT &&
>> +        spice_main_agent_test_capability(s->main,
>> VD_AGENT_CAP_GUEST_LINEEND_CRLF)) {
>> +        GError *err = NULL;
>> +
>> +        conv = spice_unix2dos((gchar*)data, len, &err);
>> +        if (err) {
>> +            g_warning("Failed to convert text line ending: %s",
>> err->message);
>> +            g_clear_error(&err);
>> +            return;
>> +        }
>> +
>> +        len = strlen(conv);
>> +    }
>> +
>>       spice_main_clipboard_selection_notify(s->main, selection, type,
>> -        gtk_selection_data_get_data(selection_data), len);
>> +                                          conv ?: data, len);
>> +    g_free(conv);
>
>
> Same question here.
>
> Otherwise this looks good.
>
> Regards,
>
> Hans

conv is non-null only if a conversion was done.

-- 
Marc-André Lureau