[Spice-commits] 2 commits - common/marshaller.c configure.ac tests/Makefile.am tests/test-marshallers.c tests/test-marshallers.h tests/test-marshallers.proto

Christophe Fergau teuf at kemper.freedesktop.org
Tue Sep 15 08:55:17 PDT 2015


 common/marshaller.c          |    2 -
 configure.ac                 |    1 
 tests/Makefile.am            |   50 +++++++++++++++++++++++++++++++++++++++++++
 tests/test-marshallers.c     |   39 +++++++++++++++++++++++++++++++++
 tests/test-marshallers.h     |   11 +++++++++
 tests/test-marshallers.proto |   10 ++++++++
 6 files changed, 112 insertions(+), 1 deletion(-)

New commits:
commit 523875d8c5691bc07f4b50d89106f4f489400f0e
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Jul 31 11:51:43 2015 +0200

    Add marshaller test case
    
    This allows to test the spice_marshaller_linearize() fix which was sent
    recently.

diff --git a/configure.ac b/configure.ac
index 98311bf..3468bbf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -47,6 +47,7 @@ AC_SUBST(SPICE_COMMON_LIBS)
 AC_CONFIG_FILES([
   Makefile
   common/Makefile
+  tests/Makefile
 ])
 
 AH_BOTTOM([
diff --git a/tests/Makefile.am b/tests/Makefile.am
new file mode 100644
index 0000000..cde46e4
--- /dev/null
+++ b/tests/Makefile.am
@@ -0,0 +1,50 @@
+NULL =
+
+TESTS = test_marshallers
+check_PROGRAMS = test_marshallers
+test_marshallers_SOURCES =		\
+	generated_test_marshallers.c	\
+	generated_test_marshallers.h	\
+	test-marshallers.c		\
+	test-marshallers.h		\
+	$(NULL)
+test_marshallers_CFLAGS =		\
+	-I$(top_srcdir)/common		\
+	$(GLIB2_CFLAGS)			\
+	$(PROTOCOL_CFLAGS)		\
+	$(NULL)
+test_marshallers_LDFLAGS =				\
+	$(top_builddir)/common/libspice-common.la	\
+	$(GLIB2_LIBS)			\
+	$(NULL)
+
+# Avoid need for python(pyparsing) by end users
+TEST_MARSHALLERS =				\
+	generated_test_marshallers.c		\
+	generated_test_marshallers.h		\
+	$(NULL)
+
+BUILT_SOURCES = $(TEST_MARSHALLERS)
+
+MARSHALLERS_DEPS =							\
+	$(CODE_GENERATOR_BASEDIR)/python_modules/__init__.py		\
+	$(CODE_GENERATOR_BASEDIR)/python_modules/codegen.py		\
+	$(CODE_GENERATOR_BASEDIR)/python_modules/demarshal.py		\
+	$(CODE_GENERATOR_BASEDIR)/python_modules/marshal.py		\
+	$(CODE_GENERATOR_BASEDIR)/python_modules/ptypes.py		\
+	$(CODE_GENERATOR_BASEDIR)/python_modules/spice_parser.py	\
+	$(CODE_GENERATOR_BASEDIR)/spice_codegen.py			\
+	$(NULL)
+
+# Note despite being autogenerated these are not part of CLEANFILES, they are
+# actually a part of EXTRA_DIST, to avoid the need for pyparser by end users
+generated_test_marshallers.c: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(CODE_GENERATOR_BASEDIR)/spice_codegen.py --generate-marshallers --server --include test-marshallers.h $< $@ >/dev/null
+generated_test_marshallers.h: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(CODE_GENERATOR_BASEDIR)/spice_codegen.py --generate-marshallers --server --include test-marshallers.h -H $< $@ >/dev/null
+
+EXTRA_DIST =				\
+	$(TEST_MARSHALLERS)		\
+	$(NULL)
+
+-include $(top_srcdir)/git.mk
diff --git a/tests/test-marshallers.c b/tests/test-marshallers.c
new file mode 100644
index 0000000..e3e4d09
--- /dev/null
+++ b/tests/test-marshallers.c
@@ -0,0 +1,39 @@
+#include <glib.h>
+#include <string.h>
+#include <marshaller.h>
+
+#include <generated_test_marshallers.h>
+
+static uint8_t expected_data[] = { 0x02, 0x00, 0x00, 0x00, /* data_size */
+                                   0x08, 0x00, 0x00, 0x00, /* data offset */
+                                   0xef, 0xcd, 0xab, 0x90, 0x78, 0x56, 0x34, 0x12, /* data */
+                                   0xef, 0xcd, 0xab, 0x90, 0x78, 0x56, 0x34, 0x12, /* data */
+};
+
+int main(int argc, char **argv)
+{
+    SpiceMarshaller *marshaller;
+    SpiceMsgMainShortDataSubMarshall *msg;
+    size_t len;
+    int free_res;
+    uint8_t *data;
+
+    msg = spice_malloc0(sizeof(SpiceMsgMainShortDataSubMarshall) + 2 * sizeof(uint64_t));
+    msg->data_size = 2;
+    msg->data[0] = 0x1234567890abcdef;
+    msg->data[1] = 0x1234567890abcdef;
+
+    marshaller = spice_marshaller_new();
+    spice_marshall_msg_main_ShortDataSubMarshall(marshaller, msg);
+    spice_marshaller_flush(marshaller);
+    data = spice_marshaller_linearize(marshaller, 0, &len, &free_res);
+    g_assert_cmpint(len, ==, G_N_ELEMENTS(expected_data));
+    g_assert_true(memcmp(data, expected_data, len) == 0);
+    if (free_res) {
+        free(data);
+    }
+    spice_marshaller_destroy(marshaller);
+    free(msg);
+
+    return 0;
+}
diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
new file mode 100644
index 0000000..9cd34c7
--- /dev/null
+++ b/tests/test-marshallers.h
@@ -0,0 +1,11 @@
+#include <stdint.h>
+
+#ifndef _H_TEST_MARSHALLERS
+
+typedef struct {
+    uint32_t data_size;
+    uint64_t data[];
+} SpiceMsgMainShortDataSubMarshall;
+
+#endif /* _H_TEST_MARSHALLERS */
+
diff --git a/tests/test-marshallers.proto b/tests/test-marshallers.proto
new file mode 100644
index 0000000..e360b09
--- /dev/null
+++ b/tests/test-marshallers.proto
@@ -0,0 +1,10 @@
+channel TestChannel {
+   message {
+      uint32 data_size;
+      uint64 *data[data_size] @marshall;
+   } ShortDataSubMarshall;
+};
+
+protocol Spice {
+    TestChannel main = 1;
+};
commit 7b146745eed1b4b5719362870576996512f08513
Author: Javier Celaya <javier.celaya at flexvdi.com>
Date:   Mon Jul 27 13:27:14 2015 +0200

    Fix linearization of several marshallers with one item
    
    The linearization optimization that avoids copying only one item must
    check that there are no further marshallers in the chain.
    
    Just to be clear, we are trying to marshall a message like this:
    
    message {
        uint32 data_size;
        uint64 *data[data_size] @marshall;
    } SomeData;
    
    Where the data field points to an array in dynamic memory. Marshalling
    and demarshalling functions look good. The marshalling function creates
    a submarshaller for the data field and links it to the root marshaller.
    But when it comes to sending the data through the wire, only the
    data_size field gets sent. We have observed that, in
    spice_marshaller_linearize, execution enters into the optimization that
    avoids copying the data when the root marshaller only has one item, but
    it ignores the following marshallers in the list. Checking if there are
    more marshallers fixes the problem.

diff --git a/common/marshaller.c b/common/marshaller.c
index 55b9d51..cedb321 100644
--- a/common/marshaller.c
+++ b/common/marshaller.c
@@ -419,7 +419,7 @@ uint8_t *spice_marshaller_linearize(SpiceMarshaller *m, size_t skip_bytes,
     /* Only supported for root marshaller */
     assert(m->data->marshallers == m);
 
-    if (m->n_items == 1) {
+    if (m->n_items == 1 && m->next == NULL) {
         *free_res = FALSE;
         if (m->items[0].len <= skip_bytes) {
             *len = 0;


More information about the Spice-commits mailing list