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

[Hans de Goede]

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