[Spice-commits] 6 commits - python_modules/demarshal.py python_modules/ptypes.py tests/helper-fuzzer-demarshallers.c tests/Makefile.am tests/meson.build tests/test-marshallers.c tests/test-marshallers.h tests/test-marshallers.proto

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Mon Feb 22 09:11:54 UTC 2021


 python_modules/demarshal.py         |   27 +++++++++++++++
 python_modules/ptypes.py            |   31 +++++++++++++++---
 tests/Makefile.am                   |   14 +++++---
 tests/helper-fuzzer-demarshallers.c |   22 ++++++++++--
 tests/meson.build                   |   40 +++++++++++++++++++----
 tests/test-marshallers.c            |   62 ++++++++++++++++++++++++++++++------
 tests/test-marshallers.h            |    2 +
 tests/test-marshallers.proto        |   11 ++++++
 8 files changed, 181 insertions(+), 28 deletions(-)

New commits:
commit c48140c493cf2f49b8a9c41af14f6892b7199c48
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Wed Sep 30 19:05:10 2020 +0100

    helper-fuzzer-demarshallers: Check also test demarshallers
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/tests/Makefile.am b/tests/Makefile.am
index be40bc4..69f6734 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -96,6 +96,7 @@ test_utils_LDADD =					\
 noinst_PROGRAMS += helper_fuzzer_demarshallers
 helper_fuzzer_demarshallers_SOURCES =	\
 	helper-fuzzer-demarshallers.c	\
+	generated_test_demarshallers.c	\
 	$(NULL)
 helper_fuzzer_demarshallers_CFLAGS =	\
 	-I$(top_srcdir)			\
@@ -132,14 +133,18 @@ MARSHALLERS_DEPS =					\
 # 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) $(top_srcdir)/spice_codegen.py --generate-marshallers --server --include test-marshallers.h $< $@ >/dev/null
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --suffix=_test \
+	--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
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --suffix=_test \
+	--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 \
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --suffix=_test \
+	--generate-demarshallers --client --include test-marshallers.h \
 	--generated-declaration-file generated_test_messages.h $< $@ >/dev/null
 generated_test_enums.h: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEPS)
-	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py -e $< $@ >/dev/null
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --suffix=_test \
+	-e $< $@ >/dev/null
 
 EXTRA_DIST =				\
 	$(TEST_MARSHALLERS)		\
diff --git a/tests/helper-fuzzer-demarshallers.c b/tests/helper-fuzzer-demarshallers.c
index c8e49ce..99c67e9 100644
--- a/tests/helper-fuzzer-demarshallers.c
+++ b/tests/helper-fuzzer-demarshallers.c
@@ -48,7 +48,7 @@ spice_parse_t(uint8_t *message_start, uint8_t *message_end,
               uint32_t channel, uint16_t message_type, SPICE_GNUC_UNUSED int minor,
               size_t *size_out, message_destructor_t *free_message);
 
-spice_parse_t spice_parse_msg, spice_parse_reply;
+spice_parse_t spice_parse_msg, spice_parse_reply, spice_parse_msg_test;
 
 int main(int argc, char **argv)
 {
@@ -78,9 +78,23 @@ int main(int argc, char **argv)
     uint8_t channel;
     READ(channel);
 
-    // Low bit select client or server
-    spice_parse_t *parse_func = channel & 1 ? spice_parse_reply : spice_parse_msg;
-    channel >>= 1;
+    // Low bits select client/server and test
+    spice_parse_t *parse_func;
+    switch (channel & 3) {
+    case 0:
+        parse_func = spice_parse_reply;
+        break;
+    case 1:
+        parse_func = spice_parse_msg;
+        break;
+    case 3:
+        parse_func = spice_parse_msg_test;
+        break;
+    default:
+        fclose(f);
+        return 1;
+    }
+    channel >>= 2;
 
     uint16_t type;
     READ(type);
diff --git a/tests/meson.build b/tests/meson.build
index c5df468..debce75 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -24,25 +24,46 @@ if spice_common_generate_client_code or spice_common_generate_server_code
   test_proto = files('test-marshallers.proto')
 
   test_marshallers_sources = ['test-marshallers.c', 'test-marshallers.h']
+  generated_marshallers = []
 
   targets = [
-      ['test_marshallers', test_proto, 'generated_test_marshallers.c', ['--generate-marshallers', '--server', '--include', 'test-marshallers.h', '@INPUT@', '@OUTPUT@']],
-      ['test_marshallers_h', test_proto, 'generated_test_marshallers.h', ['--generate-marshallers', '--server', '--include', 'test-marshallers.h', '-H', '@INPUT@', '@OUTPUT@']],
+      ['test_marshallers', test_proto, 'generated_test_marshallers.c', [
+        '--suffix=_test',
+        '--generate-marshallers',
+        '--server',
+        '--include', 'test-marshallers.h',
+        '@INPUT@', '@OUTPUT@'
+      ]],
+      ['test_marshallers_h', test_proto, 'generated_test_marshallers.h', [
+        '--suffix=_test',
+        '--generate-marshallers',
+        '--server',
+        '--include', 'test-marshallers.h',
+        '-H',
+        '@INPUT@', '@OUTPUT@'
+      ]],
       ['test_demarshallers', test_proto, 'generated_test_demarshallers.c', [
+        '--suffix=_test',
         '--generate-demarshallers',
         '--client',
         '--include', 'test-marshallers.h',
         '--generated-declaration-file', 'generated_test_messages.h',
         '@INPUT@', '@OUTPUT@'
       ]],
-      ['test_enums_h', test_proto, 'generated_test_enums.h', ['-e', '@INPUT@', '@OUTPUT@']],
+      ['test_enums_h', test_proto, 'generated_test_enums.h', [
+        '--suffix=_test',
+        '-e',
+        '@INPUT@', '@OUTPUT@'
+      ]],
   ]
 
   foreach t : targets
     cmd = [python, spice_codegen] + t[3]
-    test_marshallers_sources += custom_target(t[0], input: t[1], output : t[2], command: cmd, depend_files : spice_codegen_files)
+    generated_marshallers += custom_target(t[0], input: t[1], output : t[2], command: cmd, depend_files : spice_codegen_files)
   endforeach
 
+  test_marshallers_sources += generated_marshallers
+
   test('test_marshallers',
        executable('test_marshallers', test_marshallers_sources,
                   dependencies : spice_common_dep,
@@ -64,7 +85,8 @@ endif
 # helper_fuzzer_demarshallers
 #
 if spice_common_generate_client_code and spice_common_generate_server_code
-  executable('helper_fuzzer_demarshallers', 'helper-fuzzer-demarshallers.c',
+  executable('helper_fuzzer_demarshallers',
+             ['helper-fuzzer-demarshallers.c'] + generated_marshallers,
              dependencies : [tests_deps, spice_common_server_dep, spice_common_client_dep],
              install : false)
 endif
diff --git a/tests/test-marshallers.c b/tests/test-marshallers.c
index d8c34d6..579483b 100644
--- a/tests/test-marshallers.c
+++ b/tests/test-marshallers.c
@@ -30,6 +30,9 @@
 
 #define NUM_CHANNELS 3u
 
+spice_parse_channel_func_t
+spice_get_server_channel_parser_test(uint32_t channel, unsigned int *max_message_type);
+
 static void test_overflow(SpiceMarshaller *m)
 {
     SpiceMsgChannels *msg;
@@ -63,7 +66,7 @@ static void test_overflow(SpiceMarshaller *m)
     *((uint32_t *) data) = 0x80000002u;
 
     // extract the message
-    func = spice_get_server_channel_parser(SPICE_CHANNEL_MAIN, &max_message_type);
+    func = spice_get_server_channel_parser_test(SPICE_CHANNEL_MAIN, &max_message_type);
     g_assert_nonnull(func);
     out = func(data, data+len, SPICE_MSG_MAIN_CHANNELS_LIST, 0, &len, &free_output);
     g_assert_null(out);
@@ -86,9 +89,9 @@ static uint8_t expected_data[] = { 123, /* dummy byte */
 };
 
 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);
+uint8_t * spice_parse_msg_test(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);
 
 static void test_zerolen1(void)
 {
@@ -110,8 +113,8 @@ static void test_zerolen1(void)
 
     // demarshal array with @zero_terminated data
     SpiceMsgMainZeroLen1 *msg = (SpiceMsgMainZeroLen1 *)
-        spice_parse_msg(data, data + sizeof(data), SPICE_CHANNEL_MAIN, SPICE_MSG_MAIN_ZEROLEN1,
-                        0, &msg_len, &free_message);
+        spice_parse_msg_test(data, data + sizeof(data), SPICE_CHANNEL_MAIN, SPICE_MSG_MAIN_ZEROLEN1,
+                             0, &msg_len, &free_message);
 
     g_assert_nonnull(msg);
     g_assert_cmpmem(msg->txt1, 5, "data", 5);
@@ -156,8 +159,9 @@ int main(void)
 
     // test demarshaller
     msg = (SpiceMsgMainShortDataSubMarshall *)
-        spice_parse_msg(data, data + len, SPICE_CHANNEL_MAIN, SPICE_MSG_MAIN_SHORTDATASUBMARSHALL,
-                        0, &msg_len, &free_message);
+        spice_parse_msg_test(data, data + len, SPICE_CHANNEL_MAIN,
+                             SPICE_MSG_MAIN_SHORTDATASUBMARSHALL,
+                             0, &msg_len, &free_message);
 
     g_assert_nonnull(msg);
     g_assert_cmpint(msg->dummy_byte, ==, 123);
@@ -191,8 +195,8 @@ int main(void)
     len = 4;
     data = g_new0(uint8_t, len);
     memset(data, 0, len);
-    msg = (SpiceMsgMainShortDataSubMarshall *) spice_parse_msg(data, data + len, 1, 3, 0,
-                                                               &msg_len, &free_message);
+    msg = (SpiceMsgMainShortDataSubMarshall *) spice_parse_msg_test(data, data + len, 1, 3, 0,
+                                                                    &msg_len, &free_message);
     g_assert_null(msg);
     g_free(data);
 
commit 6b662331f7bef5d9058029885184ea1f15e9ef0d
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Fri Sep 18 08:49:02 2020 +0100

    codegen: Handle zero_terminated attribute in demashaller
    
    Make sure the output array is zero terminated to simplify
    code using the output structures.
    
    Changed in generated code:
    
        diff -ru gen/generated_client_demarshallers.c common/generated_client_demarshallers.c
        --- gen/generated_client_demarshallers.c    2021-02-21 15:13:42.004307087 +0000
        +++ common/generated_client_demarshallers.c 2021-02-21 15:13:58.916513426 +0000
        @@ -565,15 +565,24 @@
             return NULL;
         }
    
        -static uint8_t * parse_array_uint8(uint8_t *message_start, SPICE_GNUC_UNUSED uint8_t *message_end, uint8_t *struct_data, PointerInfo *this_ptr_info)
        +static uint8_t * parse_array_uint8_terminated(uint8_t *message_start, SPICE_GNUC_UNUSED uint8_t *message_end, uint8_t *struct_data, PointerInfo *this_ptr_info)
         {
             uint8_t *in = message_start + this_ptr_info->offset;
             uint8_t *end;
    
             end = struct_data;
             memcpy(end, in, this_ptr_info->nelements);
        +#if defined(__GNUC__)
        +#pragma GCC diagnostic push
        +#pragma GCC diagnostic ignored "-Wstringop-overflow"
        +#endif
        +    ((char *) (end))[this_ptr_info->nelements] = 0;
        +#if defined(__GNUC__)
        +#pragma GCC diagnostic pop
        +#endif
             in += this_ptr_info->nelements;
             end += this_ptr_info->nelements;
        +    end += 1;
             return end;
         }
    
        @@ -622,7 +631,8 @@
                     dst_info_host_data__array__nelements = host_size__value;
    
                     dst_info_host_data__array__nw_size = dst_info_host_data__array__nelements;
        -            dst_info_host_data__array__mem_size = sizeof(uint8_t) * dst_info_host_data__array__nelements;
        +            dst_info_host_data__array__mem_size = sizeof(uint8_t) * dst_info_host_data__array__nelements + sizeof(uint8_t);
        +            dst_info_host_data__array__mem_size = SPICE_ALIGN(dst_info_host_data__array__mem_size, 4);
                     if (SPICE_UNLIKELY(dst_info_host_data__array__nw_size > (uintptr_t) (message_end - message_start - host_data__value))) {
                         goto error;
                     }
        @@ -650,7 +660,8 @@
                     dst_info_cert_subject_data__array__nelements = cert_subject_size__value;
    
                     dst_info_cert_subject_data__array__nw_size = dst_info_cert_subject_data__array__nelements;
        -            dst_info_cert_subject_data__array__mem_size = sizeof(uint8_t) * dst_info_cert_subject_data__array__nelements;
        +            dst_info_cert_subject_data__array__mem_size = sizeof(uint8_t) * dst_info_cert_subject_data__array__nelements + sizeof(uint8_t);
        +            dst_info_cert_subject_data__array__mem_size = SPICE_ALIGN(dst_info_cert_subject_data__array__mem_size, 4);
                     if (SPICE_UNLIKELY(dst_info_cert_subject_data__array__nw_size > (uintptr_t) (message_end - message_start - cert_subject_data__value))) {
                         goto error;
                     }
        @@ -685,14 +696,14 @@
                 out->dst_info.sport = consume_uint16(&in);
                 out->dst_info.host_size = consume_uint32(&in);
                 ptr_info[n_ptr].offset = consume_uint32(&in);
        -        ptr_info[n_ptr].parse = parse_array_uint8;
        +        ptr_info[n_ptr].parse = parse_array_uint8_terminated;
                 ptr_info[n_ptr].dest = (void **)&out->dst_info.host_data;
                 host_data__array__nelements = out->dst_info.host_size;
                 ptr_info[n_ptr].nelements = host_data__array__nelements;
                 n_ptr++;
                 out->dst_info.cert_subject_size = consume_uint32(&in);
                 ptr_info[n_ptr].offset = consume_uint32(&in);
        -        ptr_info[n_ptr].parse = parse_array_uint8;
        +        ptr_info[n_ptr].parse = parse_array_uint8_terminated;
                 ptr_info[n_ptr].dest = (void **)&out->dst_info.cert_subject_data;
                 cert_subject_data__array__nelements = out->dst_info.cert_subject_size;
                 ptr_info[n_ptr].nelements = cert_subject_data__array__nelements;
        @@ -1050,7 +1061,8 @@
                 host_data__array__nelements = host_size__value;
    
                 host_data__array__nw_size = host_data__array__nelements;
        -        host_data__array__mem_size = sizeof(uint8_t) * host_data__array__nelements;
        +        host_data__array__mem_size = sizeof(uint8_t) * host_data__array__nelements + sizeof(uint8_t);
        +        host_data__array__mem_size = SPICE_ALIGN(host_data__array__mem_size, 4);
                 if (SPICE_UNLIKELY(host_data__array__nw_size > (uintptr_t) (message_end - message_start - host_data__value))) {
                     goto error;
                 }
        @@ -1078,7 +1090,8 @@
                 cert_subject_data__array__nelements = cert_subject_size__value;
    
                 cert_subject_data__array__nw_size = cert_subject_data__array__nelements;
        -        cert_subject_data__array__mem_size = sizeof(uint8_t) * cert_subject_data__array__nelements;
        +        cert_subject_data__array__mem_size = sizeof(uint8_t) * cert_subject_data__array__nelements + sizeof(uint8_t);
        +        cert_subject_data__array__mem_size = SPICE_ALIGN(cert_subject_data__array__mem_size, 4);
                 if (SPICE_UNLIKELY(cert_subject_data__array__nw_size > (uintptr_t) (message_end - message_start - cert_subject_data__value))) {
                     goto error;
                 }
        @@ -1107,13 +1120,13 @@
             out->sport = consume_uint16(&in);
             out->host_size = consume_uint32(&in);
             ptr_info[n_ptr].offset = consume_uint32(&in);
        -    ptr_info[n_ptr].parse = parse_array_uint8;
        +    ptr_info[n_ptr].parse = parse_array_uint8_terminated;
             ptr_info[n_ptr].dest = (void **)&out->host_data;
             ptr_info[n_ptr].nelements = host_data__array__nelements;
             n_ptr++;
             out->cert_subject_size = consume_uint32(&in);
             ptr_info[n_ptr].offset = consume_uint32(&in);
        -    ptr_info[n_ptr].parse = parse_array_uint8;
        +    ptr_info[n_ptr].parse = parse_array_uint8_terminated;
             ptr_info[n_ptr].dest = (void **)&out->cert_subject_data;
             ptr_info[n_ptr].nelements = cert_subject_data__array__nelements;
             n_ptr++;
        @@ -1338,7 +1351,8 @@
                     dst_info_host_data__array__nelements = host_size__value;
    
                     dst_info_host_data__array__nw_size = dst_info_host_data__array__nelements;
        -            dst_info_host_data__array__mem_size = sizeof(uint8_t) * dst_info_host_data__array__nelements;
        +            dst_info_host_data__array__mem_size = sizeof(uint8_t) * dst_info_host_data__array__nelements + sizeof(uint8_t);
        +            dst_info_host_data__array__mem_size = SPICE_ALIGN(dst_info_host_data__array__mem_size, 4);
                     if (SPICE_UNLIKELY(dst_info_host_data__array__nw_size > (uintptr_t) (message_end - message_start - host_data__value))) {
                         goto error;
                     }
        @@ -1366,7 +1380,8 @@
                     dst_info_cert_subject_data__array__nelements = cert_subject_size__value;
    
                     dst_info_cert_subject_data__array__nw_size = dst_info_cert_subject_data__array__nelements;
        -            dst_info_cert_subject_data__array__mem_size = sizeof(uint8_t) * dst_info_cert_subject_data__array__nelements;
        +            dst_info_cert_subject_data__array__mem_size = sizeof(uint8_t) * dst_info_cert_subject_data__array__nelements + sizeof(uint8_t);
        +            dst_info_cert_subject_data__array__mem_size = SPICE_ALIGN(dst_info_cert_subject_data__array__mem_size, 4);
                     if (SPICE_UNLIKELY(dst_info_cert_subject_data__array__nw_size > (uintptr_t) (message_end - message_start - cert_subject_data__value))) {
                         goto error;
                     }
        @@ -1401,14 +1416,14 @@
                 out->dst_info.sport = consume_uint16(&in);
                 out->dst_info.host_size = consume_uint32(&in);
                 ptr_info[n_ptr].offset = consume_uint32(&in);
        -        ptr_info[n_ptr].parse = parse_array_uint8;
        +        ptr_info[n_ptr].parse = parse_array_uint8_terminated;
                 ptr_info[n_ptr].dest = (void **)&out->dst_info.host_data;
                 host_data__array__nelements = out->dst_info.host_size;
                 ptr_info[n_ptr].nelements = host_data__array__nelements;
                 n_ptr++;
                 out->dst_info.cert_subject_size = consume_uint32(&in);
                 ptr_info[n_ptr].offset = consume_uint32(&in);
        -        ptr_info[n_ptr].parse = parse_array_uint8;
        +        ptr_info[n_ptr].parse = parse_array_uint8_terminated;
                 ptr_info[n_ptr].dest = (void **)&out->dst_info.cert_subject_data;
                 cert_subject_data__array__nelements = out->dst_info.cert_subject_size;
                 ptr_info[n_ptr].nelements = cert_subject_data__array__nelements;
        @@ -7582,7 +7597,8 @@
                 name__array__nelements = name_size__value;
    
                 name__array__nw_size = name__array__nelements;
        -        name__array__mem_size = sizeof(uint8_t) * name__array__nelements;
        +        name__array__mem_size = sizeof(uint8_t) * name__array__nelements + sizeof(uint8_t);
        +        name__array__mem_size = SPICE_ALIGN(name__array__mem_size, 4);
                 if (SPICE_UNLIKELY(name__array__nw_size > (uintptr_t) (message_end - message_start - name__value))) {
                     goto error;
                 }
        @@ -7609,7 +7625,7 @@
    
             out->name_size = consume_uint32(&in);
             ptr_info[n_ptr].offset = consume_uint32(&in);
        -    ptr_info[n_ptr].parse = parse_array_uint8;
        +    ptr_info[n_ptr].parse = parse_array_uint8_terminated;
             ptr_info[n_ptr].dest = (void **)&out->name;
             ptr_info[n_ptr].nelements = name__array__nelements;
             n_ptr++;
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index efc4c99..2a111be 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -384,6 +384,10 @@ def write_validate_array_item(writer, container, item, scope, parent_scope, star
         # TODO: Overflow check the multiplication
         if array.has_attr("ptr_array"):
             writer.assign(mem_size, "sizeof(void *) + SPICE_ALIGN(%s * %s, 4)" % (element_type.sizeof(), nelements))
+        elif array.has_attr('zero_terminated'):
+            # don't use +1 here to avoid possible integer overflow or suboptimizations
+            writer.assign(mem_size, "%s * %s + %s" % (element_type.sizeof(), nelements, element_type.sizeof()))
+            writer.assign(mem_size, "SPICE_ALIGN(%s, 4)" % mem_size);
         else:
             writer.assign(mem_size, "%s * %s" % (element_type.sizeof(), nelements))
         want_mem_size = False
@@ -724,7 +728,10 @@ def write_switch_parser(writer, container, switch, dest, scope):
 
 def write_parse_ptr_function(writer, target_type):
     if target_type.is_array():
-        parse_function = "parse_array_%s" % target_type.element_type.primitive_type()
+        if target_type.has_attr("zero_terminated"):
+            parse_function = "parse_array_%s_terminated" % target_type.element_type.primitive_type()
+        else:
+            parse_function = "parse_array_%s" % target_type.element_type.primitive_type()
     else:
         parse_function = "parse_struct_%s" % target_type.c_type()
     if writer.is_generated("parser", parse_function):
@@ -787,10 +794,28 @@ def write_array_parser(writer, member, nelements, array, dest, scope):
         if array.has_attr("ptr_array"):
             raise Exception("Attribute ptr_array not supported for arrays of int8/uint8")
         writer.statement("memcpy(%s, in, %s)" % (array_start, nelements))
+        if array.has_attr("zero_terminated"):
+            indentation = writer.indentation
+            writer.indentation = 0;
+            writer.writeln("#if defined(__GNUC__)")
+            writer.writeln("#pragma GCC diagnostic push")
+            writer.writeln("#pragma GCC diagnostic ignored \"-Wstringop-overflow\"")
+            writer.writeln("#endif")
+            writer.indentation = indentation;
+            writer.assign("((char *) (%s))[%s]" % (array_start, nelements), 0)
+            writer.indentation = 0;
+            writer.writeln("#if defined(__GNUC__)")
+            writer.writeln("#pragma GCC diagnostic pop")
+            writer.writeln("#endif")
+            writer.indentation = indentation;
         writer.increment("in", nelements)
         if at_end:
             writer.increment("end", nelements)
+            if array.has_attr("zero_terminated"):
+                writer.increment("end", 1)
     else:
+        if array.has_attr("zero_terminated"):
+            raise Exception("Attribute zero_terminated specified for wrong array")
         with writer.index() as index:
             if member:
                 array_pos = "%s[%s]" % (array_start, index)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index e3eb8fd..d68dc67 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -553,6 +553,8 @@ class ArrayType(Type):
             return writer.writeln('%s *%s[0];' % (self.c_type(), name))
         if member.has_end_attr():
             return writer.writeln('%s %s[0];' % (self.c_type(), name))
+        if self.is_constant_length() and self.has_attr("zero_terminated"):
+            return writer.writeln('%s %s[%s];' % (self.c_type(), name, self.size + 1))
         if self.is_constant_length():
             return writer.writeln('%s %s[%s];' % (self.c_type(), name, self.size))
         if self.is_identifier_length():
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 05d9ba2..be40bc4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -136,7 +136,8 @@ generated_test_marshallers.c: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEP
 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
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-demarshallers --client --include test-marshallers.h \
+	--generated-declaration-file generated_test_messages.h $< $@ >/dev/null
 generated_test_enums.h: $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEPS)
 	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py -e $< $@ >/dev/null
 
diff --git a/tests/meson.build b/tests/meson.build
index bd53565..c5df468 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -28,7 +28,13 @@ if spice_common_generate_client_code or spice_common_generate_server_code
   targets = [
       ['test_marshallers', test_proto, 'generated_test_marshallers.c', ['--generate-marshallers', '--server', '--include', 'test-marshallers.h', '@INPUT@', '@OUTPUT@']],
       ['test_marshallers_h', test_proto, 'generated_test_marshallers.h', ['--generate-marshallers', '--server', '--include', 'test-marshallers.h', '-H', '@INPUT@', '@OUTPUT@']],
-      ['test_demarshallers', test_proto, 'generated_test_demarshallers.c', ['--generate-demarshallers', '--client', '--include', 'test-marshallers.h', '@INPUT@', '@OUTPUT@']],
+      ['test_demarshallers', test_proto, 'generated_test_demarshallers.c', [
+        '--generate-demarshallers',
+        '--client',
+        '--include', 'test-marshallers.h',
+        '--generated-declaration-file', 'generated_test_messages.h',
+        '@INPUT@', '@OUTPUT@'
+      ]],
       ['test_enums_h', test_proto, 'generated_test_enums.h', ['-e', '@INPUT@', '@OUTPUT@']],
   ]
 
diff --git a/tests/test-marshallers.c b/tests/test-marshallers.c
index afa681d..d8c34d6 100644
--- a/tests/test-marshallers.c
+++ b/tests/test-marshallers.c
@@ -1,5 +1,5 @@
 /*
-   Copyright (C) 2015-2018 Red Hat, Inc.
+   Copyright (C) 2015-2021 Red Hat, Inc.
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
@@ -90,6 +90,44 @@ 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);
 
+static void test_zerolen1(void)
+{
+    static uint8_t data[] = {
+        'd', 'a', 't', 'a', // txt1
+        123, // sep1
+        4, 0, 0, 0, // len
+        26, 0, 0, 0, // ptr to txt2
+        'n','e','k','o', // txt3
+        12, 13, 14, 15, // n
+        3, 0, // txt4_len
+        'b','a','r', // txt4
+        'f', 'o', 'o', '!', // string
+        'x', 'x', // garbage at the end
+    };
+
+    size_t msg_len;
+    message_destructor_t free_message;
+
+    // demarshal array with @zero_terminated data
+    SpiceMsgMainZeroLen1 *msg = (SpiceMsgMainZeroLen1 *)
+        spice_parse_msg(data, data + sizeof(data), SPICE_CHANNEL_MAIN, SPICE_MSG_MAIN_ZEROLEN1,
+                        0, &msg_len, &free_message);
+
+    g_assert_nonnull(msg);
+    g_assert_cmpmem(msg->txt1, 5, "data", 5);
+    g_assert_cmpint(msg->sep1, ==, 123);
+    g_assert_cmpint(msg->txt2_len, ==, 4);
+    g_assert_cmpint(msg->n , ==, 0x0f0e0d0c);
+    g_assert_nonnull(msg->txt2);
+    g_assert_cmpmem(msg->txt2, 5, "foo!", 5);
+    g_assert_nonnull(msg->txt3);
+    g_assert_cmpmem(msg->txt3, 5, "neko", 5);
+    g_assert_cmpint(msg->txt4_len, ==, 3);
+    g_assert_cmpmem(msg->txt4, 4, "bar", 4);
+
+    free_message((uint8_t *) msg);
+}
+
 int main(void)
 {
     SpiceMarshaller *marshaller;
@@ -135,6 +173,8 @@ int main(void)
     }
     spice_marshaller_reset(marshaller);
 
+    test_zerolen1();
+
     SpiceMsgMainZeroes msg_zeroes = { 0x0102 };
 
     spice_marshall_msg_main_Zeroes(marshaller, &msg_zeroes);
diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
index 8ca736e..8470e38 100644
--- a/tests/test-marshallers.h
+++ b/tests/test-marshallers.h
@@ -3,6 +3,8 @@
 
 #include <stdint.h>
 
+#include "generated_test_messages.h"
+
 typedef struct {
     uint32_t data_size;
     uint8_t dummy_byte;
diff --git a/tests/test-marshallers.proto b/tests/test-marshallers.proto
index eabd487..6ca2d4c 100644
--- a/tests/test-marshallers.proto
+++ b/tests/test-marshallers.proto
@@ -24,6 +24,17 @@ channel TestChannel {
         uint32 dummy[2];
         uint8 data[] @end;
     } LenMessage;
+
+    message {
+        uint8 txt1[4] @zero_terminated;
+        uint8 sep1;
+        uint32 txt2_len;
+        uint8 *txt2[txt2_len] @zero_terminated;
+        uint8 txt3[txt2_len] @to_ptr @zero_terminated;
+        uint32 n;
+        uint16 txt4_len;
+        uint8 txt4[txt4_len] @end @zero_terminated;
+    } @declare ZeroLen1;
 };
 
 protocol Spice {
commit bd15c96f545b100c1c872cf57b45fa0b54481cc9
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Wed Sep 23 10:30:11 2020 +0100

    codegen: Propagate zero_terminated attribute
    
    Currently the attribute is not used.
    However we would like to make sure when present that the array
    is always zero terminated to simplify usages after demarshalling.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 32c3e16..e3eb8fd 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -18,7 +18,7 @@ def get_named_types():
 # only to attributes that affect pointer or array attributes, as these
 # are member local types, unlike e.g. a Struct that may be used by
 # other members
-propagated_attributes=["ptr_array", "nonnull", "chunk"]
+propagated_attributes=["ptr_array", "nonnull", "chunk", "zero_terminated"]
 
 valid_attributes_generic=set([
     # embedded/appended at the end of the resulting C structure
commit a627a14af0430c2c76ae2951d9b977ad1c8dabcb
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Fri Sep 18 08:52:38 2020 +0100

    codegen: Propagate attributes to element under pointers
    
    Current propagated attributes are propagated only from member to
    member type.
    But for pointers you probably want to use propagated attribus to
    underlying type (like an array for instance).
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 71bb7f7..32c3e16 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -641,6 +641,8 @@ class Member(Containee):
         for i in propagated_attributes:
             if self.has_attr(i):
                 self.member_type.attributes[i] = self.attributes[i]
+                if self.member_type.is_pointer():
+                    self.member_type.target_type.attributes[i] = self.attributes[i]
         return self
 
     def contains_member(self, member):
commit 6838f30079e6401f89597f1209f09bdea257ce28
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Fri Sep 25 16:37:13 2020 +0100

    codegen: Add a check to array type
    
    If the size is not constant the array has to be allocated in some
    way in the output and so there must be a specification for the
    output (as default is write into the C structure all data).
    The only exceptions are when the length is constant (in this case
    a constant length array in the C structure is used) or a pointer
    (in this case the pointer allocate the array).
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 91bd0cd..71bb7f7 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -501,6 +501,16 @@ class ArrayType(Type):
         return self.element_type.c_type()
 
     def check_valid(self, member):
+        # If the size is not constant the array has to be allocated in some
+        # way in the output and so there must be a specification for the
+        # output (as default is write into the C structure all data).
+        # The only exceptions are when the length is constant (in this case
+        # a constant length array in the C structure is used) or a pointer
+        # (in this case the pointer allocate the array).
+        if (not self.is_constant_length()
+            and len(output_attributes.intersection(member.attributes.keys())) == 0
+            and not member.member_type.is_pointer()):
+            raise Exception("Array length must be a constant or some output specifiers must be set")
         # These attribute corresponds to specific structure size
         if member.has_attr("chunk") or member.has_attr("as_ptr"):
             return
commit e49301ebd157057864dbd98dae558a3e7c6c4e9b
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Fri Sep 25 16:33:46 2020 +0100

    codegen: Make "output_attrs" variable global
    
    Allows to reuse it.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index eba26a6..91bd0cd 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -86,6 +86,16 @@ attributes_with_arguments=set([
     'virtual',
 ])
 
+# these attributes specify output format, only one can be set
+output_attributes = set([
+    'end',
+    'to_ptr',
+    'as_ptr',
+    'ptr_array',
+    'zero',
+    'chunk',
+])
+
 def fix_attributes(attribute_list, valid_attributes=valid_attributes_generic):
     attrs = {}
     for attr in attribute_list:
@@ -100,9 +110,8 @@ def fix_attributes(attribute_list, valid_attributes=valid_attributes_generic):
             raise Exception("Attribute %s has more than 1 argument" % name)
         attrs[name] = lst
 
-    # these attributes specify output format, only one can be set
-    output_attrs = set(['end', 'to_ptr', 'as_ptr', 'ptr_array', 'zero', 'chunk'])
-    if len(output_attrs.intersection(attrs.keys())) > 1:
+    # only one output attribute can be set
+    if len(output_attributes.intersection(attrs.keys())) > 1:
         raise Exception("Multiple output type attributes specified %s" % output_attrs)
 
     return attrs


More information about the Spice-commits mailing list