[Spice-commits] 5 commits - configure.ac gtk/channel-main.c gtk/Makefile.am gtk/spice-gtk-session.c gtk/spice-util.c gtk/spice-util-priv.h Makefile.am tests/Makefile.am tests/util.c

Marc-André Lureau elmarco at kemper.freedesktop.org
Sat Aug 24 09:23:00 PDT 2013


 Makefile.am             |    2 
 configure.ac            |    1 
 gtk/Makefile.am         |    2 
 gtk/channel-main.c      |   73 +---------------------------------
 gtk/spice-gtk-session.c |   58 +++++++++++++++++++++++----
 gtk/spice-util-priv.h   |    2 
 gtk/spice-util.c        |  103 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile.am       |   18 ++++++++
 tests/util.c            |   89 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 271 insertions(+), 77 deletions(-)

New commits:
commit a5f9eb76442ca2fa346772684f069ca3aa7a69b0
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Sat Aug 24 00:03:07 2013 +0200

    tests: add some dos2unix tests
    
    This is probably not exhaustive enough, but better than nothing.

diff --git a/Makefile.am b/Makefile.am
index ffa1649..ab10f5f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,7 +1,7 @@
 ACLOCAL_AMFLAGS = -I m4
 NULL =
 
-SUBDIRS = spice-common gtk po doc data
+SUBDIRS = spice-common gtk po doc data tests
 
 if HAVE_INTROSPECTION
 if WITH_VALA
diff --git a/configure.ac b/configure.ac
index 1235f4a..74738a3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -685,6 +685,7 @@ gtk/controller/Makefile
 doc/Makefile
 doc/reference/Makefile
 vapi/Makefile
+tests/Makefile
 ])
 
 dnl ==========================================================================
diff --git a/gtk/spice-util.c b/gtk/spice-util.c
index 745033e..e02fc4d 100644
--- a/gtk/spice-util.c
+++ b/gtk/spice-util.c
@@ -22,6 +22,7 @@
 
 #include <stdlib.h>
 #include <string.h>
+#include <glib.h>
 #include <glib-object.h>
 #include "spice-util-priv.h"
 #include "spice-util.h"
diff --git a/tests/Makefile.am b/tests/Makefile.am
new file mode 100644
index 0000000..9510e2c
--- /dev/null
+++ b/tests/Makefile.am
@@ -0,0 +1,18 @@
+NULL =
+
+noinst_PROGRAMS = util
+TESTS = $(noinst_PROGRAMS)
+
+AM_CPPFLAGS =					\
+	$(GIO_CFLAGS) -I$(top_srcdir)/gtk	\
+	-DG_LOG_DOMAIN=\"GSpice\"		\
+	$(NULL)
+AM_LDFLAGS = $(GIO_LIBS)
+
+util_SOURCES =					\
+	$(top_srcdir)/gtk/spice-util-priv.h	\
+	$(top_srcdir)/gtk/spice-util.c		\
+	$(top_srcdir)/gtk/spice-util.h  	\
+	util.c					\
+	$(NULL)
+
diff --git a/tests/util.c b/tests/util.c
new file mode 100644
index 0000000..86109aa
--- /dev/null
+++ b/tests/util.c
@@ -0,0 +1,89 @@
+#include <glib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include "spice-util-priv.h"
+
+enum {
+    DOS2UNIX = 1 << 0,
+    UNIX2DOS = 1 << 1,
+};
+
+static const struct {
+    const gchar *d;
+    const gchar *u;
+    glong flags;
+} dosunix[] = {
+    { "", "", DOS2UNIX|UNIX2DOS },
+    { "a", "a", DOS2UNIX|UNIX2DOS },
+    { "\r\n", "\n", DOS2UNIX|UNIX2DOS },
+    { "\r\n\r\n", "\n\n", DOS2UNIX|UNIX2DOS },
+    { "a\r\n", "a\n", DOS2UNIX|UNIX2DOS },
+    { "a\r\n\r\n", "a\n\n", DOS2UNIX|UNIX2DOS },
+    { "\r\n\r\na\r\n\r\n", "\n\na\n\n", DOS2UNIX|UNIX2DOS },
+    { "1\r\n\r\na\r\n\r\n2", "1\n\na\n\n2", DOS2UNIX|UNIX2DOS },
+    { "\n", "\n", DOS2UNIX },
+    { "\n\n", "\n\n", DOS2UNIX },
+    { "\r\n", "\r\n", UNIX2DOS },
+    { "\r\r\n", "\r\r\n", UNIX2DOS },
+    { "é\r\né", "é\né", DOS2UNIX|UNIX2DOS },
+    { "\r\né\r\né\r\n", "\né\né\n", DOS2UNIX|UNIX2DOS }
+    /* TODO: add some utf8 test cases */
+};
+
+static void test_dos2unix(void)
+{
+    GError *err = NULL;
+    gchar *tmp;
+    int i;
+
+    for (i = 0; i < G_N_ELEMENTS(dosunix); i++) {
+        if (!(dosunix[i].flags & DOS2UNIX))
+            continue;
+
+        tmp = spice_dos2unix(dosunix[i].d, -1, &err);
+        g_assert_cmpstr(tmp, ==, dosunix[i].u);
+        g_assert_no_error(err);
+        g_free(tmp);
+
+        /* including ending \0 */
+        tmp = spice_dos2unix(dosunix[i].d, strlen(dosunix[i].d) + 1, &err);
+        g_assert_cmpstr(tmp, ==, dosunix[i].u);
+        g_assert_no_error(err);
+        g_free(tmp);
+    }
+}
+
+static void test_unix2dos(void)
+{
+    GError *err = NULL;
+    gchar *tmp;
+    int i;
+
+    for (i = 0; i < G_N_ELEMENTS(dosunix); i++) {
+        if (!(dosunix[i].flags & UNIX2DOS))
+            continue;
+
+        tmp = spice_unix2dos(dosunix[i].u, -1, &err);
+        g_assert_cmpstr(tmp, ==, dosunix[i].d);
+        g_assert_no_error(err);
+        g_free(tmp);
+
+        /* including ending \0 */
+        tmp = spice_unix2dos(dosunix[i].u, strlen(dosunix[i].u) + 1, &err);
+        g_assert_cmpstr(tmp, ==, dosunix[i].d);
+        g_assert_no_error(err);
+        g_free(tmp);
+    }
+}
+
+int main(int argc, char* argv[])
+{
+  g_test_init(&argc, &argv, NULL);
+
+  g_test_add_func("/util/dos2unix", test_dos2unix);
+  g_test_add_func("/util/unix2dos", test_unix2dos);
+
+  return g_test_run ();
+}
commit d3b08c9a3583ee52d7acf7cf30abfab7aadad7d1
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Fri Aug 23 17:02:10 2013 +0200

    gtk: handle clipboard CRLF conversion
    
    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.

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);
+    } 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);
 }
 
 static gboolean clipboard_request(SpiceMainChannel *main, guint selection,
commit 75f1ea3ee9c4dbd6c5f27896caee07792bbdbba4
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Fri Aug 23 15:47:01 2013 +0200

    util: add unix2dos and dos2unix
    
    Convert line endings from/to LF/CRLF.

diff --git a/gtk/spice-util-priv.h b/gtk/spice-util-priv.h
index ee5a42d..cc559dc 100644
--- a/gtk/spice-util-priv.h
+++ b/gtk/spice-util-priv.h
@@ -29,6 +29,8 @@ gboolean spice_strv_contains(const GStrv strv, const gchar *str);
 gchar* spice_uuid_to_string(const guint8 uuid[16]);
 const gchar* spice_yes_no(gboolean value);
 guint16 spice_make_scancode(guint scancode, gboolean release);
+gchar* spice_unix2dos(const gchar *str, gssize len, GError **error);
+gchar* spice_dos2unix(const gchar *str, gssize len, GError **error);
 
 #if GLIB_CHECK_VERSION(2,32,0)
 #define STATIC_MUTEX            GMutex
diff --git a/gtk/spice-util.c b/gtk/spice-util.c
index 774a145..745033e 100644
--- a/gtk/spice-util.c
+++ b/gtk/spice-util.c
@@ -19,6 +19,7 @@
 #ifdef HAVE_CONFIG_H
 # include "config.h"
 #endif
+
 #include <stdlib.h>
 #include <string.h>
 #include <glib-object.h>
@@ -245,3 +246,104 @@ guint16 spice_make_scancode(guint scancode, gboolean release)
 
     g_return_val_if_reached(0);
 }
+
+typedef enum {
+    NEWLINE_TYPE_LF,
+    NEWLINE_TYPE_CR_LF
+} NewlineType;
+
+static gssize get_line(const gchar *str, gsize len,
+                       NewlineType type, gsize *nl_len,
+                       GError **error)
+{
+    const gchar *p, *endl;
+    gsize nl = 0;
+
+    endl = (type == NEWLINE_TYPE_CR_LF) ? "\r\n" : "\n";
+    p = g_strstr_len(str, len, endl);
+    if (p) {
+        len = p - str;
+        nl = strlen(endl);
+    }
+
+    *nl_len = nl;
+    return len;
+}
+
+
+static gchar* spice_convert_newlines(const gchar *str, gssize len,
+                                     NewlineType from,
+                                     NewlineType to,
+                                     GError **error)
+{
+    GError *err = NULL;
+    gssize length;
+    gsize nl;
+    GString *output;
+    gboolean free_segment = FALSE;
+    gint i;
+
+    g_return_val_if_fail(str != NULL, NULL);
+    g_return_val_if_fail(len >= -1, NULL);
+    g_return_val_if_fail(error == NULL || *error == NULL, NULL);
+    /* only 2 supported combinations */
+    g_return_val_if_fail((from == NEWLINE_TYPE_LF &&
+                          to == NEWLINE_TYPE_CR_LF) ||
+                         (from == NEWLINE_TYPE_CR_LF &&
+                          to == NEWLINE_TYPE_LF), NULL);
+
+    if (len == -1)
+        len = strlen(str);
+    /* sometime we get \0 terminated strings, skip that, or it fails
+       to utf8 validate line with \0 end */
+    else if (len > 0 && str[len-1] == 0)
+        len -= 1;
+
+    /* allocate worst case, if it's small enough, we don't care much,
+     * if it's big, malloc will put us in mmap'd region, and we can
+     * over allocate.
+     */
+    output = g_string_sized_new(len * 2 + 1);
+
+    for (i = 0; i < len; i += length + nl) {
+        length = get_line(str + i, len - i, from, &nl, error);
+        if (length < 0)
+            break;
+
+        g_string_append_len(output, str + i, length);
+
+        if (nl) {
+            /* let's not double \r if it's already in the line */
+            if (to == NEWLINE_TYPE_CR_LF &&
+                output->str[output->len - 1] != '\r')
+                g_string_append_c(output, '\r');
+
+            g_string_append_c(output, '\n');
+        }
+    }
+
+    if (err) {
+        g_propagate_error(error, err);
+        free_segment = TRUE;
+    }
+
+    return g_string_free(output, free_segment);
+}
+
+G_GNUC_INTERNAL
+gchar* spice_dos2unix(const gchar *str, gssize len, GError **error)
+{
+    return spice_convert_newlines(str, len,
+                                  NEWLINE_TYPE_CR_LF,
+                                  NEWLINE_TYPE_LF,
+                                  error);
+}
+
+G_GNUC_INTERNAL
+gchar* spice_unix2dos(const gchar *str, gssize len, GError **error)
+{
+    return spice_convert_newlines(str, len,
+                                  NEWLINE_TYPE_LF,
+                                  NEWLINE_TYPE_CR_LF,
+                                  error);
+}
commit ce07cfdf4874b0d323c6a3a1c83a0ac28c24c68d
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Fri Aug 23 15:50:39 2013 +0200

    Revert "channel-main: Convert text line-endings if necessary (rhbz#752350)"
    
    This implementation assumes that the toolkit doesn't do any conversion
    on its own. This is actually not the case, gtk+ converts line endings
    already on windows.  It would be pretty ugly to do conversions back and
    forth at different levels. So the channel implementation shouldn't
    try to do conversion, and implementation must be done at higher level,
    where the actual system clipboard implementation is known (at
    gtk-session for instance)
    
    Since this code hasn't been officially released, let's revert it and
    implement a better crlf conversion handling in gtk-session in the
    following commits.
    
    This reverts commit e45a446a9981ad4adaeff9c885962a8c6140333e.

diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index b9e0da2..b58af52 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -1181,24 +1181,6 @@ static void agent_announce_caps(SpiceMainChannel *channel)
 #define HAS_CLIPBOARD_SELECTION(c) \
     VD_AGENT_HAS_CAPABILITY((c)->agent_caps, G_N_ELEMENTS((c)->agent_caps), VD_AGENT_CAP_CLIPBOARD_SELECTION)
 
-#define GUEST_LINEEND_LF(c) \
-    VD_AGENT_HAS_CAPABILITY((c)->agent_caps, G_N_ELEMENTS((c)->agent_caps), VD_AGENT_CAP_GUEST_LINEEND_LF)
-
-#define GUEST_LINEEND_CRLF(c) \
-    VD_AGENT_HAS_CAPABILITY((c)->agent_caps, G_N_ELEMENTS((c)->agent_caps), VD_AGENT_CAP_GUEST_LINEEND_CRLF)
-
-#ifdef G_OS_UNIX
-#define CLIENT_LINEEND_LF 1
-#else
-#define CLIENT_LINEEND_LF 0
-#endif
-
-#ifdef G_OS_WIN32
-#define CLIENT_LINEEND_CRLF 1
-#else
-#define CLIENT_LINEEND_CRLF 0
-#endif
-
 /* any context: the message is not flushed immediately,
    you can wakeup() the channel coroutine or send_msg_queue() */
 static void agent_clipboard_grab(SpiceMainChannel *channel, guint selection,
@@ -1769,29 +1751,6 @@ static void file_xfer_handle_status(SpiceMainChannel *channel,
     file_xfer_completed(task, error);
 }
 
-/* any context */
-static guchar *convert_lineend(const guchar *in, gsize *size,
-                               const gchar *from, const gchar *to)
-{
-    gchar *nul_terminated, **split, *out;
-
-    /* Nul-terminate */
-    nul_terminated = g_malloc(*size + 1);
-    memcpy(nul_terminated, in, *size);
-    nul_terminated[*size] = 0;
-
-    /* Convert */
-    split = g_strsplit(nul_terminated, from, -1);
-    out = g_strjoinv(to, split);
-    *size = strlen(out);
-
-    /* Clean-up */
-    g_strfreev(split);
-    g_free(nul_terminated);
-
-    return (guchar *)out;
-}
-
 /* coroutine context */
 static void main_agent_handle_msg(SpiceChannel *channel,
                                   VDAgentMessage *msg, gpointer payload)
@@ -1850,22 +1809,12 @@ static void main_agent_handle_msg(SpiceChannel *channel,
     case VD_AGENT_CLIPBOARD:
     {
         VDAgentClipboard *cb = payload;
-        guchar *data = cb->data;
-        gsize size = msg->size - sizeof(VDAgentClipboard);
-        if (cb->type == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
-            if (GUEST_LINEEND_LF(c) && CLIENT_LINEEND_CRLF)
-                data = convert_lineend(data, &size, "\n", "\r\n");
-            if (GUEST_LINEEND_CRLF(c) && CLIENT_LINEEND_LF)
-                data = convert_lineend(data, &size, "\r\n", "\n");
-        }
         emit_main_context(channel, SPICE_MAIN_CLIPBOARD_SELECTION, selection,
-                          cb->type, data, size);
+                          cb->type, cb->data, msg->size - sizeof(VDAgentClipboard));
 
-        if (selection == VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD)
+       if (selection == VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD)
             emit_main_context(channel, SPICE_MAIN_CLIPBOARD,
-                              cb->type, data, size);
-        if (data != cb->data)
-            g_free(data);
+                              cb->type, cb->data, msg->size - sizeof(VDAgentClipboard));
         break;
     }
     case VD_AGENT_CLIPBOARD_GRAB:
@@ -2605,27 +2554,13 @@ void spice_main_clipboard_notify(SpiceMainChannel *channel,
  * Since: 0.6
  **/
 void spice_main_clipboard_selection_notify(SpiceMainChannel *channel, guint selection,
-                                           guint32 type, const guchar *_data, size_t _size)
+                                           guint32 type, const guchar *data, size_t size)
 {
-    const guchar *data = _data;
-    gsize size = _size;
-
     g_return_if_fail(channel != NULL);
     g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel));
 
-    SpiceMainChannelPrivate *c = channel->priv;
-
-    if (type == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
-        if (CLIENT_LINEEND_CRLF && GUEST_LINEEND_LF(c))
-            data = convert_lineend(data, &size, "\r\n", "\n");
-        if (CLIENT_LINEEND_LF && GUEST_LINEEND_CRLF(c))
-            data = convert_lineend(data, &size, "\n", "\r\n");
-    }
     agent_clipboard_notify(channel, selection, type, data, size);
     spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
-
-    if (data != _data)
-        g_free((guchar *)data);
 }
 
 /**
commit 2c462def6acc5dc7b16786b5f86a03202e67f0a0
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Fri Aug 23 17:22:13 2013 +0200

    gtk: remove dead code

diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
index 47c7d36..68777eb 100644
--- a/gtk/spice-gtk-session.c
+++ b/gtk/spice-gtk-session.c
@@ -367,7 +367,6 @@ static gint get_selection_from_clipboard(SpiceGtkSessionPrivate *s,
 static const struct {
     const char  *xatom;
     uint32_t    vdagent;
-    uint32_t    flags;
 } atom2agent[] = {
     {
         .vdagent = VD_AGENT_CLIPBOARD_UTF8_TEXT,
@@ -660,7 +659,6 @@ static gboolean clipboard_grab(SpiceMainChannel *main, guint selection,
                 found = TRUE;
                 g_return_val_if_fail(i < SPICE_N_ELEMENTS(atom2agent), FALSE);
                 targets[i].target = (gchar*)atom2agent[m].xatom;
-                targets[i].flags = 0;
                 targets[i].info = m;
                 target_selected[m] = TRUE;
                 i += 1;


More information about the Spice-commits mailing list