[Spice-commits] 8 commits - common/lines.c common/lz_compress_tmpl.c python_modules/demarshal.py python_modules/marshal.py python_modules/ptypes.py spice.proto tests/Makefile.am tests/test-overflow.c

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Mon May 14 15:43:20 UTC 2018


 common/lines.c              |    2 -
 common/lz_compress_tmpl.c   |    3 -
 python_modules/demarshal.py |   52 ++++++++++++--------------
 python_modules/marshal.py   |    7 +--
 python_modules/ptypes.py    |   15 ++-----
 spice.proto                 |   17 +++++---
 tests/Makefile.am           |   14 +++++++
 tests/test-overflow.c       |   86 ++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 146 insertions(+), 50 deletions(-)

New commits:
commit 617be0f74b88ce53d84d417c00696b8c1630b6ec
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Jun 19 19:08:55 2015 +0100

    Avoid integer overflow computing image sizes
    
    Use always 64, sizes can be 32x32.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index 7e73985..8d3f5cb 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -346,13 +346,12 @@ def write_validate_array_item(writer, container, item, scope, parent_scope, star
         rows = array.size[3]
         width_v = write_read_primitive(writer, start, container, width, scope)
         rows_v = write_read_primitive(writer, start, container, rows, scope)
-        # TODO: Handle multiplication overflow
         if bpp == 8:
-            writer.assign(nelements, "%s * %s" % (width_v, rows_v))
+            writer.assign(nelements, "(uint64_t) %s * %s" % (width_v, rows_v))
         elif bpp == 1:
-            writer.assign(nelements, "((%s + 7) / 8 ) * %s" % (width_v, rows_v))
+            writer.assign(nelements, "(((uint64_t) %s + 7U) / 8U ) * %s" % (width_v, rows_v))
         else:
-            writer.assign(nelements, "((%s * %s + 7) / 8 ) * %s" % (bpp, width_v, rows_v))
+            writer.assign(nelements, "((%sU * (uint64_t) %s + 7U) / 8U ) * %s" % (bpp, width_v, rows_v))
     elif array.is_bytes_length():
         is_byte_size = True
         v = write_read_primitive(writer, start, container, array.size[1], scope)
@@ -713,13 +712,12 @@ def read_array_len(writer, prefix, array, dest, scope, is_ptr):
         rows = array.size[3]
         width_v = dest.get_ref(width)
         rows_v = dest.get_ref(rows)
-        # TODO: Handle multiplication overflow
         if bpp == 8:
-            writer.assign(nelements, "%s * %s" % (width_v, rows_v))
+            writer.assign(nelements, "((uint64_t) %s * %s)" % (width_v, rows_v))
         elif bpp == 1:
-            writer.assign(nelements, "((%s + 7) / 8 ) * %s" % (width_v, rows_v))
+            writer.assign(nelements, "(((uint64_t) %s + 7U) / 8U ) * %s" % (width_v, rows_v))
         else:
-            writer.assign(nelements, "((%s * %s + 7) / 8 ) * %s" % (bpp, width_v, rows_v))
+            writer.assign(nelements, "((%sU * (uint64_t) %s + 7U) / 8U ) * %s" % (bpp, width_v, rows_v))
     elif array.is_bytes_length():
         writer.assign(nelements, dest.get_ref(array.size[2]))
     else:
diff --git a/python_modules/marshal.py b/python_modules/marshal.py
index 402273c..fd3416a 100644
--- a/python_modules/marshal.py
+++ b/python_modules/marshal.py
@@ -172,13 +172,12 @@ def get_array_size(array, container_src):
         rows = array.size[3]
         width_v = container_src.get_ref(width)
         rows_v = container_src.get_ref(rows)
-        # TODO: Handle multiplication overflow
         if bpp == 8:
-            return "(unsigned) (%s * %s)" % (width_v, rows_v)
+            return "((uint64_t) %s * %s)" % (width_v, rows_v)
         elif bpp == 1:
-            return "(unsigned) (((%s + 7) / 8 ) * %s)" % (width_v, rows_v)
+            return "((((uint64_t) %s + 7U) / 8U ) * %s)" % (width_v, rows_v)
         else:
-            return "(unsigned) (((%s * %s + 7) / 8 ) * %s)" % (bpp, width_v, rows_v)
+            return "((((uint64_t) %s * %s + 7U) / 8U ) * %s)" % (bpp, width_v, rows_v)
     elif array.is_bytes_length():
         return container_src.get_ref(array.size[2])
     else:
commit 420a15b776d3334b665d4015791647e2d24583d9
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Jun 19 14:42:54 2015 +0100

    Write a small test to test possible crash
    
    This small test prove a that current generated demarshaller code
    is not safe to integer overflows leading to buffer overflows.
    Actually from a quick look at the protocol it seems that client
    can't cause these overflows but server can quite easily at
    demonstrated by this test.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5abf239..d5ec1d7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -67,4 +67,18 @@ EXTRA_DIST =				\
 	test-marshallers.proto		\
 	$(NULL)
 
+TESTS += test_overflow
+test_overflow_SOURCES = test-overflow.c
+test_overflow_CFLAGS = \
+	-I$(top_srcdir) \
+	$(GLIB2_CFLAGS) \
+	$(SPICE_COMMON_CFLAGS) \
+	$(PROTOCOL_CFLAGS) \
+	$(NULL)
+test_overflow_LDADD = \
+	$(top_builddir)/common/libspice-common.la \
+	$(top_builddir)/common/libspice-common-server.la \
+	$(top_builddir)/common/libspice-common-client.la \
+	$(NULL)
+
 -include $(top_srcdir)/git.mk
diff --git a/tests/test-overflow.c b/tests/test-overflow.c
new file mode 100644
index 0000000..0a5bdd2
--- /dev/null
+++ b/tests/test-overflow.c
@@ -0,0 +1,86 @@
+/*
+   Copyright (C) 2015 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
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+
+#include <spice/enums.h>
+#include <common/marshaller.h>
+#include <common/generated_server_marshallers.h>
+#include <common/client_demarshallers.h>
+
+#define NUM_CHANNELS 3u
+
+int main(void)
+{
+    SpiceMarshaller *m;
+    SpiceMsgChannels *msg;
+    uint8_t *data, *out;
+    size_t len;
+    int to_free = 0;
+    spice_parse_channel_func_t func;
+    unsigned int max_message_type, n;
+    message_destructor_t free_output;
+
+    m = spice_marshaller_new();
+    assert(m);
+
+    msg = (SpiceMsgChannels *) malloc(sizeof(SpiceMsgChannels) +
+          NUM_CHANNELS * sizeof(SpiceChannelId));
+    assert(msg);
+
+    // build a message and marshal it
+    msg->num_of_channels = NUM_CHANNELS;
+    for (n = 0; n < NUM_CHANNELS; ++n) {
+        msg->channels[n] = (SpiceChannelId) { n + 1, n * 7 };
+    }
+    spice_marshall_msg_main_channels_list(m, msg);
+
+    // get linear data
+    data = spice_marshaller_linearize(m, 0, &len, &to_free);
+    assert(data);
+
+    printf("output len %lu\n", (unsigned long) len);
+
+    // hack: setting the number of channels in the marshalled message to a
+    // value that will cause overflow while parsing the message to make sure
+    // that the parser can handle this situation
+    *((uint32_t *) data) = 0x80000002u;
+
+    // extract the message
+    func = spice_get_server_channel_parser(SPICE_CHANNEL_MAIN, &max_message_type);
+    assert(func);
+    out = func(data, data+len, SPICE_MSG_MAIN_CHANNELS_LIST, 0, &len, &free_output);
+    assert(out == NULL);
+
+    // cleanup
+    if (to_free) {
+        free(data);
+    }
+    if (out) {
+        free_output(out);
+    }
+    free(msg);
+
+    return 0;
+}
+
commit a69fb1ec3425baf0a6dadced29669f4b708da923
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Jun 19 14:44:37 2015 +0100

    Fix integer overflows computing sizes
    
    Make code safe using both 32 and 64 bit machine.
    Consider that this code can be compiled for machines with 32 bit.
    There are some arrays length which are 32 bit.
    
    If size_t this can cause easily an overflow. For instance message_len
    sending SPICE_MSG_NOTIFY messages are 32 bit and code add a small
    constant (currently 24) before doing the test for size. Now passing
    (uint32_t) -20 as message_len would lead to a size of 4 after the
    addition. This overflow does not happen on 64 bit machine as the length
    is converted to size_t.
    
    There are also some array length where some item are bigger than 1 byte.
    For instance SPICE_MAIN_CHANNELS_LIST message have a number of channels
    and each channel is composed by 2 bytes. Now the code generated try to do
    length * 2 where length is still a 32 bit so if we put a value like
    0x80000002u we get 4 as length. This will cause an overflow as code will
    allocate very few bytes but try to fill with a huge number of elements.
    This overflow happen in both 32 and 64 bit machine.
    
    To avoid all these possible overflows this patch use only 64 bit for
    nelements (number of elements), nw_size (network size) and mem_size
    (memory size needed) checking the sizes to avoid other overflows
    (like pointers conversions under 32 bit machines).
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index da87d44..7e73985 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -101,7 +101,7 @@ def write_parser_helpers(writer):
     writer.variable_def("uint64_t", "offset")
     writer.variable_def("parse_func_t", "parse")
     writer.variable_def("void **", "dest")
-    writer.variable_def("uint32_t", "nelements")
+    writer.variable_def("uint64_t", "nelements")
     writer.end_block(semicolon=True)
 
 def write_read_primitive(writer, start, container, name, scope):
@@ -186,7 +186,7 @@ def write_validate_switch_member(writer, mprefix, container, switch_member, scop
 
             all_as_extra_size = m.is_extra_size() and want_extra_size
             if not want_mem_size and all_as_extra_size and not scope.variable_defined(item.mem_size()):
-                scope.variable_def("uint32_t", item.mem_size())
+                scope.variable_def("uint64_t", item.mem_size())
 
             sub_want_mem_size = want_mem_size or all_as_extra_size
             sub_want_extra_size = want_extra_size and not all_as_extra_size
@@ -219,7 +219,7 @@ def write_validate_struct_function(writer, struct):
     scope = writer.function(validate_function, "static intptr_t", "uint8_t *message_start, uint8_t *message_end, uint64_t offset, SPICE_GNUC_UNUSED int minor")
     scope.variable_def("uint8_t *", "start = message_start + offset")
     scope.variable_def("SPICE_GNUC_UNUSED uint8_t *", "pos")
-    scope.variable_def("size_t", "mem_size", "nw_size")
+    scope.variable_def("uint64_t", "mem_size", "nw_size")
     num_pointers = struct.get_num_pointers()
     if  num_pointers != 0:
         scope.variable_def("SPICE_GNUC_UNUSED intptr_t", "ptr_size")
@@ -236,7 +236,7 @@ def write_validate_struct_function(writer, struct):
 
     writer.newline()
     writer.comment("Check if struct fits in reported side").newline()
-    writer.error_check("start + nw_size > message_end")
+    writer.error_check("nw_size > (uintptr_t) (message_end - start)")
 
     writer.statement("return mem_size")
 
@@ -264,26 +264,26 @@ def write_validate_pointer_item(writer, container, item, scope, parent_scope, st
         # if array, need no function check
 
         if target_type.is_array():
-            writer.error_check("message_start + %s >= message_end" % v)
+            writer.error_check("%s >= (uintptr_t) (message_end - message_start)" % v)
 
 
             assert target_type.element_type.is_primitive()
 
             array_item = ItemInfo(target_type, "%s__array" % item.prefix, start)
-            scope.variable_def("uint32_t", array_item.nw_size())
+            scope.variable_def("uint64_t", array_item.nw_size())
             # don't create a variable that isn't used, fixes -Werror=unused-but-set-variable
             need_mem_size = want_mem_size or (
                 want_extra_size and not item.member.has_attr("chunk")
                 and not target_type.is_cstring_length())
             if need_mem_size:
-                scope.variable_def("uint32_t", array_item.mem_size())
+                scope.variable_def("uint64_t", array_item.mem_size())
             if target_type.is_cstring_length():
                 writer.assign(array_item.nw_size(), "spice_strnlen((char *)message_start + %s, message_end - (message_start + %s))" % (v, v))
                 writer.error_check("*(message_start + %s + %s) != 0" % (v, array_item.nw_size()))
             else:
                 write_validate_array_item(writer, container, array_item, scope, parent_scope, start,
                                           True, want_mem_size=need_mem_size, want_extra_size=False)
-                writer.error_check("message_start + %s + %s > message_end" % (v, array_item.nw_size()))
+                writer.error_check("%s + %s > (uintptr_t) (message_end - message_start)" % (v, array_item.nw_size()))
 
             if want_extra_size:
                 if item.member and item.member.has_attr("chunk"):
@@ -321,11 +321,11 @@ def write_validate_array_item(writer, container, item, scope, parent_scope, star
         nelements = "%s__nbytes" %(item.prefix)
         real_nelements = "%s__nelements" %(item.prefix)
         if not parent_scope.variable_defined(real_nelements):
-            parent_scope.variable_def("uint32_t", real_nelements)
+            parent_scope.variable_def("uint64_t", real_nelements)
     else:
         nelements = "%s__nelements" %(item.prefix)
     if not parent_scope.variable_defined(nelements):
-        parent_scope.variable_def("uint32_t", nelements)
+        parent_scope.variable_def("uint64_t", nelements)
 
     if array.is_constant_length():
         writer.assign(nelements, array.size)
@@ -420,10 +420,10 @@ def write_validate_array_item(writer, container, item, scope, parent_scope, star
     element_nw_size = element_item.nw_size()
     element_mem_size = element_item.mem_size()
     element_extra_size = element_item.extra_size()
-    scope.variable_def("uint32_t", element_nw_size)
-    scope.variable_def("uint32_t", element_mem_size)
+    scope.variable_def("uint64_t", element_nw_size)
+    scope.variable_def("uint64_t", element_mem_size)
     if want_extra_size:
-        scope.variable_def("uint32_t", element_extra_size)
+        scope.variable_def("uint64_t", element_extra_size)
 
     if want_nw_size:
         writer.assign(nw_size, 0)
@@ -556,7 +556,7 @@ def write_validate_container(writer, prefix, container, start, parent_scope, wan
         sub_want_nw_size = want_nw_size and not m.is_fixed_nw_size()
         sub_want_mem_size = m.is_extra_size() and want_mem_size
         sub_want_extra_size = not m.is_extra_size() and m.contains_extra_size()
-        defs = ["size_t"]
+        defs = ["uint64_t"]
         name = prefix_m(prefix, m)
         if sub_want_nw_size:
 
@@ -697,7 +697,7 @@ def read_array_len(writer, prefix, array, dest, scope, is_ptr):
     if dest.is_toplevel() and scope.variable_defined(nelements):
         return nelements # Already there for toplevel, need not recalculate
     element_type = array.element_type
-    scope.variable_def("uint32_t", nelements)
+    scope.variable_def("uint64_t", nelements)
     if array.is_constant_length():
         writer.assign(nelements, array.size)
     elif array.is_identifier_length():
@@ -1053,9 +1053,9 @@ def write_msg_parser(writer, message):
     parent_scope.variable_def("SPICE_GNUC_UNUSED uint8_t *", "pos")
     parent_scope.variable_def("uint8_t *", "start = message_start")
     parent_scope.variable_def("uint8_t *", "data = NULL")
-    parent_scope.variable_def("size_t", "nw_size")
+    parent_scope.variable_def("uint64_t", "nw_size")
     if want_mem_size:
-        parent_scope.variable_def("size_t", "mem_size")
+        parent_scope.variable_def("uint64_t", "mem_size")
     if not message.has_attr("nocopy"):
         parent_scope.variable_def("uint8_t *", "in", "end")
     num_pointers = message.get_num_pointers()
@@ -1073,7 +1073,7 @@ def write_msg_parser(writer, message):
     writer.newline()
 
     writer.comment("Check if message fits in reported side").newline()
-    with writer.block("if (start + nw_size > message_end)"):
+    with writer.block("if (nw_size > (uintptr_t) (message_end - start))"):
         writer.statement("return NULL")
 
     writer.newline().comment("Validated extents and calculated size").newline()
@@ -1084,7 +1084,7 @@ def write_msg_parser(writer, message):
         writer.assign("*size", "message_end - message_start")
         writer.assign("*free_message", "nofree")
     else:
-        writer.assign("data", "(uint8_t *)malloc(mem_size)")
+        writer.assign("data", "(uint8_t *)(mem_size > UINT32_MAX ? NULL : malloc(mem_size))")
         writer.error_check("data == NULL")
         writer.assign("end", "data + %s" % (msg_sizeof))
         writer.assign("in", "start").newline()
commit 75d9842e7d5a1b0f12b4d784ed41e40ecc6efcbb
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Sun Dec 24 13:25:33 2017 +0000

    lz: Move ENCODE_PIXEL for RGB24 and RGB32 to a common place
    
    The macro for both depth is the same, reuse the definition.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/common/lz_compress_tmpl.c b/common/lz_compress_tmpl.c
index 1bb5c75..69e69a6 100644
--- a/common/lz_compress_tmpl.c
+++ b/common/lz_compress_tmpl.c
@@ -125,17 +125,16 @@
 #ifdef LZ_RGB24
 #define PIXEL rgb24_pixel_t
 #define FNAME(name) lz_rgb24_##name
-#define ENCODE_PIXEL(e, pix) {encode(e, (pix).b); encode(e, (pix).g); encode(e, (pix).r);}
 #endif
 
 #ifdef LZ_RGB32
 #define PIXEL rgb32_pixel_t
 #define FNAME(name) lz_rgb32_##name
-#define ENCODE_PIXEL(e, pix) {encode(e, (pix).b); encode(e, (pix).g); encode(e, (pix).r);}
 #endif
 
 
 #if  defined(LZ_RGB24) || defined(LZ_RGB32)
+#define ENCODE_PIXEL(e, pix) {encode(e, (pix).b); encode(e, (pix).g); encode(e, (pix).r);}
 #define GET_r(pix) ((pix).r)
 #define GET_g(pix) ((pix).g)
 #define GET_b(pix) ((pix).b)
commit b98f19b1681003c6adb096ff71b29ad562db6e3a
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Sat May 5 16:43:31 2018 +0100

    protocol: Use a typedef to specify stream_id type
    
    This change does not affect generated code but make source more
    readable. Also document in a single location the range of this
    type.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/spice.proto b/spice.proto
index 6f873a2..4d916bb 100644
--- a/spice.proto
+++ b/spice.proto
@@ -4,6 +4,11 @@
 
 typedef fixed28_4 int32 @ctype(SPICE_FIXED28_4);
 
+/* IDs of the video stream messages.
+ * These IDs are in the interval [0, SPICE_MAX_NUM_STREAMS)
+ */
+typedef stream_id_t uint32;
+
 struct Point {
     int32 x;
     int32 y;
@@ -698,7 +703,7 @@ struct String {
 };
 
 struct StreamDataHeader {
-	uint32 id;
+	stream_id_t id;
 	uint32 multi_media_time;
 };
 
@@ -756,7 +761,7 @@ channel DisplayChannel : BaseChannel {
 
     message {
 	uint32 surface_id;
-	uint32 id;
+	stream_id_t id;
 	stream_flags flags;
 	video_codec_type codec_type;
 	uint64 stamp;
@@ -775,12 +780,12 @@ channel DisplayChannel : BaseChannel {
     } stream_data;
 
     message {
-	uint32 id;
+	stream_id_t id;
 	Clip clip;
     } stream_clip;
 
     message {
-	uint32 id;
+	stream_id_t id;
     } stream_destroy;
 
     Empty stream_destroy_all;
@@ -954,7 +959,7 @@ channel DisplayChannel : BaseChannel {
     } draw_composite;
 
     message {
-        uint32 stream_id;
+        stream_id_t stream_id;
         uint32 unique_id;
         uint32 max_window_size;
         uint32 timeout_ms;
@@ -986,7 +991,7 @@ channel DisplayChannel : BaseChannel {
     } init = 101;
 
     message {
-        uint32 stream_id;
+        stream_id_t stream_id;
         uint32 unique_id;
         // the mm_time of the first frame included in the report
         uint32 start_frame_mm_time;
commit fc46379b37e8dd9d06c96c074af4cc1738cacffd
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Wed May 9 16:22:16 2018 -0500

    miLineArc(): initialize edge1, edge2
    
    When compiling spice-common with meson/ninja under "release" mode, I get
    several compiler warnings about possibly-uninitialized members. For
    example:
    
        ../subprojects/spice-common/common/lines.c: In function ‘miLineArc’:
        ../subprojects/spice-common/common/lines.c:2167:17: error: ‘edge2.dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
                 edge->e += edge->dx; \
                         ^~
        ../subprojects/spice-common/common/lines.c:2426:24: note: ‘edge2.dx’ was declared here
             PolyEdgeRec edge1, edge2;
                                ^~~~~
    
    Initializing these structures to zero silences the warnings.
    
    Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/common/lines.c b/common/lines.c
index e5097c4..dadaf86 100644
--- a/common/lines.c
+++ b/common/lines.c
@@ -2423,7 +2423,7 @@ miLineArc (GCPtr pGC,
     int xorgi = 0, yorgi = 0;
     Spans spanRec;
     int n;
-    PolyEdgeRec edge1, edge2;
+    PolyEdgeRec edge1 = { 0 }, edge2 = { 0 };
     int edgey1, edgey2;
     Boolean edgeleft1, edgeleft2;
 
commit 754cd54e1a807e5e9e87402392b8be9fdb66c637
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Sun May 6 06:28:32 2018 +0100

    codegen: Removed unused get_type methods
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Lukáš Hrázký <lhrazky at redhat.com>

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index d07ee97..0f6d8d6 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -149,9 +149,6 @@ class Type:
     def has_name(self):
         return self.name != None
 
-    def get_type(self, recursive=False):
-        return self
-
     def is_primitive(self):
         return False
 
@@ -260,12 +257,6 @@ class TypeAlias(Type):
         self.the_type = the_type
         self.attributes = fix_attributes(attribute_list)
 
-    def get_type(self, recursive=False):
-        if recursive:
-            return self.the_type.get_type(True)
-        else:
-            return self.the_type
-
     def primitive_type(self):
         return self.the_type.primitive_type()
 
commit 46fa9d5efba541a99f9c4ab3e9e9e738659f9ef6
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Sun May 6 13:33:33 2018 +0100

    codegen: Add some comments
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Lukáš Hrázký <lhrazky at redhat.com>

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index d29c97a..d07ee97 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -83,6 +83,8 @@ valid_attributes=set([
     'nomarshal',
     # ??? not used by python code
     'zero_terminated',
+    # force generating marshaller code, applies to pointers which by
+    # default are not marshalled (submarshallers are generated)
     'marshall',
     # this pointer member cannot be null
     'nonnull',
@@ -97,6 +99,8 @@ valid_attributes=set([
     # the argument specifies the preprocessor define to check
     'ifdef',
     # write this member as zero on network
+    # when marshalling, a zero field is written to the network
+    # when demarshalling, the field is read from the network and discarded
     'zero',
     # specify minor version required for these members
     'minor',
@@ -124,7 +128,7 @@ attributes_with_arguments=set([
 def fix_attributes(attribute_list):
     attrs = {}
     for attr in attribute_list:
-        name = attr[0][1:]
+        name = attr[0][1:] # [1:] strips the leading '@' from the name
         lst = attr[1:]
         if not name in valid_attributes:
             raise Exception("Attribute %s not recognized" % name)


More information about the Spice-commits mailing list