[Spice-commits] 3 commits - common/marshaller.c python_modules/demarshal.py tests/Makefile.am tests/test-marshallers.c tests/test-marshallers.h tests/test-marshallers.proto
Frediano Ziglio
fziglio at kemper.freedesktop.org
Mon Sep 25 15:15:12 UTC 2017
common/marshaller.c | 57 ++++++++++++++++++++++++++++++-------------
python_modules/demarshal.py | 33 +++++++++++++++++-------
tests/Makefile.am | 4 +++
tests/test-marshallers.c | 39 ++++++++++++++++++++++++-----
tests/test-marshallers.h | 3 +-
tests/test-marshallers.proto | 1
6 files changed, 103 insertions(+), 34 deletions(-)
New commits:
commit c2f2096d900de69343bdc7a27ca7ecc22fcb1bc8
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Wed Sep 20 16:21:31 2017 +0100
test-marshallers: Test demarshalling
Generate demarshallers code and check it too.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Christophe Fergeau <cfergeau at redhat.com>
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 10033c0..5abf239 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -20,6 +20,7 @@ test_logging_LDADD = \
test_marshallers_SOURCES = \
generated_test_marshallers.c \
generated_test_marshallers.h \
+ generated_test_demarshallers.c \
test-marshallers.c \
test-marshallers.h \
$(NULL)
@@ -37,6 +38,7 @@ test_marshallers_LDADD = \
TEST_MARSHALLERS = \
generated_test_marshallers.c \
generated_test_marshallers.h \
+ generated_test_demarshallers.c \
$(NULL)
BUILT_SOURCES = $(TEST_MARSHALLERS)
@@ -57,6 +59,8 @@ generated_test_marshallers.c: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEP
$(AM_V_GEN)$(PYTHON) $(top_srcdir)/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) $(top_srcdir)/spice_codegen.py --generate-marshallers --server --include test-marshallers.h -H $< $@ >/dev/null
+generated_test_demarshallers.c: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEPS)
+ $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-demarshallers --client --include test-marshallers.h $< $@ >/dev/null
EXTRA_DIST = \
$(TEST_MARSHALLERS) \
diff --git a/tests/test-marshallers.c b/tests/test-marshallers.c
index a231b52..ae90770 100644
--- a/tests/test-marshallers.c
+++ b/tests/test-marshallers.c
@@ -15,15 +15,22 @@ static uint8_t expected_data[] = { 123, /* dummy byte */
0x21, 0x43, 0x65, 0x87, 0x09, 0xba, 0xdc, 0xfe, /* data */
};
+typedef void (*message_destructor_t)(uint8_t *message);
+uint8_t * spice_parse_msg(uint8_t *message_start, uint8_t *message_end,
+ uint32_t channel, uint16_t message_type, int minor,
+ size_t *size_out, message_destructor_t *free_message);
+
int main(int argc G_GNUC_UNUSED, char **argv G_GNUC_UNUSED)
{
SpiceMarshaller *marshaller;
SpiceMsgMainShortDataSubMarshall *msg;
- size_t len;
+ size_t len, msg_len;
int free_res;
uint8_t *data;
+ message_destructor_t free_message;
- msg = spice_malloc0(sizeof(SpiceMsgMainShortDataSubMarshall) + 2 * sizeof(uint64_t));
+ msg = g_new0(SpiceMsgMainShortDataSubMarshall, 1);
+ msg->data = g_new(uint64_t, 2);
msg->dummy_byte = 123;
msg->data_size = 2;
msg->data[0] = 0x1234567890abcdef;
@@ -35,11 +42,27 @@ int main(int argc G_GNUC_UNUSED, char **argv G_GNUC_UNUSED)
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);
+
+ g_free(msg->data);
+ g_free(msg);
+
+ // test demarshaller
+ msg = (SpiceMsgMainShortDataSubMarshall *) spice_parse_msg(data, data + len, 1, 1, 0,
+ &msg_len, &free_message);
+
+ g_assert_nonnull(msg);
+ g_assert_cmpint(msg->dummy_byte, ==, 123);
+ g_assert_cmpint(msg->data_size, ==, 2);
+ g_assert_nonnull(msg->data);
+ g_assert_cmpint(msg->data[0], ==, 0x1234567890abcdef);
+ g_assert_cmpint(msg->data[1], ==, 0xfedcba0987654321);
+
+ free_message((uint8_t *) msg);
+
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
index 371fcdc..7e5a0b2 100644
--- a/tests/test-marshallers.h
+++ b/tests/test-marshallers.h
@@ -5,7 +5,7 @@
typedef struct {
uint32_t data_size;
uint8_t dummy_byte;
- uint64_t data[];
+ uint64_t *data;
} SpiceMsgMainShortDataSubMarshall;
#endif /* _H_TEST_MARSHALLERS */
commit c25c75b552d840a277e9e76d13a6055309f3bb6a
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Wed Sep 20 15:38:38 2017 +0100
test-marshallers: Use unaligned structure
Allows to test for bad performance on some systems.
For instance on ARMv6/ARMv7 which does not support by default
64 bit unaligned read/write this can be checked on Linux
using /proc/cpu/alignment file.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Christophe Fergeau <cfergeau at redhat.com>
diff --git a/tests/test-marshallers.c b/tests/test-marshallers.c
index 734a2bb..a231b52 100644
--- a/tests/test-marshallers.c
+++ b/tests/test-marshallers.c
@@ -8,10 +8,11 @@
#define g_assert_true g_assert
#endif
-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 */
+static uint8_t expected_data[] = { 123, /* dummy byte */
+ 0x02, 0x00, 0x00, 0x00, /* data_size */
+ 0x09, 0x00, 0x00, 0x00, /* data offset */
0xef, 0xcd, 0xab, 0x90, 0x78, 0x56, 0x34, 0x12, /* data */
+ 0x21, 0x43, 0x65, 0x87, 0x09, 0xba, 0xdc, 0xfe, /* data */
};
int main(int argc G_GNUC_UNUSED, char **argv G_GNUC_UNUSED)
@@ -23,9 +24,10 @@ int main(int argc G_GNUC_UNUSED, char **argv G_GNUC_UNUSED)
uint8_t *data;
msg = spice_malloc0(sizeof(SpiceMsgMainShortDataSubMarshall) + 2 * sizeof(uint64_t));
+ msg->dummy_byte = 123;
msg->data_size = 2;
msg->data[0] = 0x1234567890abcdef;
- msg->data[1] = 0x1234567890abcdef;
+ msg->data[1] = 0xfedcba0987654321;
marshaller = spice_marshaller_new();
spice_marshall_msg_main_ShortDataSubMarshall(marshaller, msg);
diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
index 9cd34c7..371fcdc 100644
--- a/tests/test-marshallers.h
+++ b/tests/test-marshallers.h
@@ -4,6 +4,7 @@
typedef struct {
uint32_t data_size;
+ uint8_t dummy_byte;
uint64_t data[];
} SpiceMsgMainShortDataSubMarshall;
diff --git a/tests/test-marshallers.proto b/tests/test-marshallers.proto
index e360b09..68b5822 100644
--- a/tests/test-marshallers.proto
+++ b/tests/test-marshallers.proto
@@ -1,5 +1,6 @@
channel TestChannel {
message {
+ uint8 dummy_byte; // so structure is not aligned
uint32 data_size;
uint64 *data[data_size] @marshall;
} ShortDataSubMarshall;
commit 74e50b57ae05116be4e27c13f1dacf8716d47a64
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Mon Sep 11 11:10:58 2017 +0100
Make the compiler work out better way to write unaligned memory
Instead of assuming that the system can safely do unaligned access
to memory use packed structures to allow the compiler generate
best code possible.
A packed structure tells the compiler to not leave padding inside it
and that the structure can be unaligned so any field can be unaligned
having to generate proper access code based on architecture.
For instance ARM7 can use unaligned access but not for 64 bit
numbers (currently these accesses are emulated by Linux kernel
with obvious performance consequences).
This changes the current methods from:
#ifdef WORDS_BIGENDIAN
#define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(*((uint32_t *)(ptr))))
#define write_uint32(ptr, val) *(uint32_t *)(ptr) = SPICE_BYTESWAP32((uint32_t)val)
#else
#define read_uint32(ptr) (*((uint32_t *)(ptr)))
#define write_uint32(ptr, val) (*((uint32_t *)(ptr))) = val
#endif
to:
#include <spice/start-packed.h>
typedef struct SPICE_ATTR_PACKED {
uint32_t v;
} uint32_unaligned_t;
#include <spice/end-packed.h>
#ifdef WORDS_BIGENDIAN
#define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(((uint32_unaligned_t *)(ptr))->v))
#define write_uint32(ptr, val) ((uint32_unaligned_t *)(ptr))->v = SPICE_BYTESWAP32((uint32_t)val)
#else
#define read_uint32(ptr) (((uint32_unaligned_t *)(ptr))->v)
#define write_uint32(ptr, val) (((uint32_unaligned_t *)(ptr))->v) = val
#endif
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Christophe Fergeau <cfergeau at redhat.com>
diff --git a/common/marshaller.c b/common/marshaller.c
index 2fccd52..adfb8cd 100644
--- a/common/marshaller.c
+++ b/common/marshaller.c
@@ -28,24 +28,49 @@
#include <unistd.h>
#include <stdio.h>
+#include <spice/start-packed.h>
+typedef struct SPICE_ATTR_PACKED {
+ int16_t val;
+} int16_unaligned_t;
+
+typedef struct SPICE_ATTR_PACKED {
+ uint16_t val;
+} uint16_unaligned_t;
+
+typedef struct SPICE_ATTR_PACKED {
+ int32_t val;
+} int32_unaligned_t;
+
+typedef struct SPICE_ATTR_PACKED {
+ uint32_t val;
+} uint32_unaligned_t;
+
+typedef struct SPICE_ATTR_PACKED {
+ int64_t val;
+} int64_unaligned_t;
+
+typedef struct SPICE_ATTR_PACKED {
+ uint64_t val;
+} uint64_unaligned_t;
+#include <spice/end-packed.h>
+
+#define write_int8(ptr,v) (*(int8_t *)(ptr) = v)
+#define write_uint8(ptr,v) (*(uint8_t *)(ptr) = v)
+
#ifdef WORDS_BIGENDIAN
-#define write_int8(ptr,v) (*((int8_t *)(ptr)) = v)
-#define write_uint8(ptr,v) (*((uint8_t *)(ptr)) = v)
-#define write_int16(ptr,v) (*((int16_t *)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v)))
-#define write_uint16(ptr,v) (*((uint16_t *)(ptr)) = SPICE_BYTESWAP16((uint16_t)(v)))
-#define write_int32(ptr,v) (*((int32_t *)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v)))
-#define write_uint32(ptr,v) (*((uint32_t *)(ptr)) = SPICE_BYTESWAP32((uint32_t)(v)))
-#define write_int64(ptr,v) (*((int64_t *)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v)))
-#define write_uint64(ptr,v) (*((uint64_t *)(ptr)) = SPICE_BYTESWAP64((uint64_t)(v)))
+#define write_int16(ptr,v) (((uint16_unaligned_t *)(ptr))->val = SPICE_BYTESWAP16((uint16_t)(v)))
+#define write_uint16(ptr,v) (((uint16_unaligned_t *)(ptr))->val = SPICE_BYTESWAP16((uint16_t)(v)))
+#define write_int32(ptr,v) (((uint32_unaligned_t *)(ptr))->val = SPICE_BYTESWAP32((uint32_t)(v)))
+#define write_uint32(ptr,v) (((uint32_unaligned_t *)(ptr))->val = SPICE_BYTESWAP32((uint32_t)(v)))
+#define write_int64(ptr,v) (((uint64_unaligned_t *)(ptr))->val = SPICE_BYTESWAP64((uint64_t)(v)))
+#define write_uint64(ptr,v) (((uint64_unaligned_t *)(ptr))->val = SPICE_BYTESWAP64((uint64_t)(v)))
#else
-#define write_int8(ptr,v) (*((int8_t *)(ptr)) = v)
-#define write_uint8(ptr,v) (*((uint8_t *)(ptr)) = v)
-#define write_int16(ptr,v) (*((int16_t *)(ptr)) = v)
-#define write_uint16(ptr,v) (*((uint16_t *)(ptr)) = v)
-#define write_int32(ptr,v) (*((int32_t *)(ptr)) = v)
-#define write_uint32(ptr,v) (*((uint32_t *)(ptr)) = v)
-#define write_int64(ptr,v) (*((int64_t *)(ptr)) = v)
-#define write_uint64(ptr,v) (*((uint64_t *)(ptr)) = v)
+#define write_int16(ptr,v) (((int16_unaligned_t *)(ptr))->val = v)
+#define write_uint16(ptr,v) (((uint16_unaligned_t *)(ptr))->val = v)
+#define write_int32(ptr,v) (((int32_unaligned_t *)(ptr))->val = v)
+#define write_uint32(ptr,v) (((uint32_unaligned_t *)(ptr))->val = v)
+#define write_int64(ptr,v) (((int64_unaligned_t *)(ptr))->val = v)
+#define write_uint64(ptr,v) (((uint64_unaligned_t *)(ptr))->val = v)
#endif
typedef struct {
diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index 1ea131d..da87d44 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -40,24 +40,37 @@ def write_parser_helpers(writer):
writer = writer.function_helper()
+ writer.writeln("#include <spice/start-packed.h>")
+ for size in [16, 32, 64]:
+ for sign in ["", "u"]:
+ type = "%sint%d" % (sign, size)
+ writer.begin_block("typedef struct SPICE_ATTR_PACKED")
+ writer.variable_def("%s_t" % type, "v")
+ writer.end_block(newline=False)
+ writer.writeln(" %s_unaligned_t;" % type)
+ writer.writeln("#include <spice/end-packed.h>")
+ writer.newline()
+
+ for sign in ["", "u"]:
+ type = "%sint8" % sign
+ writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" % type)
+ writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr) = val" % (type))
+ writer.newline()
+
writer.writeln("#ifdef WORDS_BIGENDIAN")
- for size in [8, 16, 32, 64]:
+ for size in [16, 32, 64]:
for sign in ["", "u"]:
utype = "uint%d" % (size)
type = "%sint%d" % (sign, size)
swap = "SPICE_BYTESWAP%d" % size
- if size == 8:
- writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" % type)
- writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr) = val" % (type))
- else:
- writer.macro("read_%s" % type, "ptr", "((%s_t)%s(*((%s_t *)(ptr))))" % (type, swap, utype))
- writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr) = %s((%s_t)val)" % (utype, swap, utype))
+ writer.macro("read_%s" % type, "ptr", "((%s_t)%s(((%s_unaligned_t *)(ptr))->v))" % (type, swap, utype))
+ writer.macro("write_%s" % type, "ptr, val", "((%s_unaligned_t *)(ptr))->v = %s((%s_t)val)" % (utype, swap, utype))
writer.writeln("#else")
- for size in [8, 16, 32, 64]:
+ for size in [16, 32, 64]:
for sign in ["", "u"]:
type = "%sint%d" % (sign, size)
- writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" % type)
- writer.macro("write_%s" % type, "ptr, val", "(*((%s_t *)(ptr))) = val" % type)
+ writer.macro("read_%s" % type, "ptr", "(((%s_unaligned_t *)(ptr))->v)" % type)
+ writer.macro("write_%s" % type, "ptr, val", "(((%s_unaligned_t *)(ptr))->v) = val" % type)
writer.writeln("#endif")
for size in [8, 16, 32, 64]:
More information about the Spice-commits
mailing list