[Spice-devel] [PATCH spice-common 3/3] codegen: Remove minor attribute

Frediano Ziglio fziglio at redhat.com
Tue Oct 9 16:24:03 UTC 2018


The idea in version 1 of the protocol was to extend it using the
minor version.
However this was replaced by the usage of capabilities and the minor
attribute (which was not much used in version 1) was abandoned in
version 2.
This patch create a big difference in the code generated but only
because the minor version was passed between all possible functions.
Note that external function, for compatibility retains the minor
argument.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 docs/spice_protocol.txt     |  5 ----
 python_modules/demarshal.py | 59 +++++++------------------------------
 python_modules/ptypes.py    | 52 --------------------------------
 3 files changed, 10 insertions(+), 106 deletions(-)

diff --git a/docs/spice_protocol.txt b/docs/spice_protocol.txt
index 421393d..b205743 100644
--- a/docs/spice_protocol.txt
+++ b/docs/spice_protocol.txt
@@ -445,11 +445,6 @@ zero
 
 TODO
 
-minor
-~~~~~
-
-TODO
-
 virtual
 ~~~~~~~
 
diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index 5209272..6fe3c66 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -92,8 +92,8 @@ def write_parser_helpers(writer):
     writer.newline()
     writer.statement("typedef struct PointerInfo PointerInfo")
     writer.statement("typedef void (*message_destructor_t)(uint8_t *message)")
-    writer.statement("typedef uint8_t * (*parse_func_t)(uint8_t *message_start, uint8_t *message_end, uint8_t *struct_data, PointerInfo *ptr_info, int minor)")
-    writer.statement("typedef uint8_t * (*parse_msg_func_t)(uint8_t *message_start, uint8_t *message_end, int minor, size_t *size_out, message_destructor_t *free_message)")
+    writer.statement("typedef uint8_t * (*parse_func_t)(uint8_t *message_start, uint8_t *message_end, uint8_t *struct_data, PointerInfo *ptr_info)")
+    writer.statement("typedef uint8_t * (*parse_msg_func_t)(uint8_t *message_start, uint8_t *message_end, size_t *size_out, message_destructor_t *free_message)")
     writer.statement("typedef uint8_t * (*spice_parse_channel_func_t)(uint8_t *message_start, uint8_t *message_end, uint16_t message_type, int minor, size_t *size_out, message_destructor_t *free_message)")
 
     writer.newline()
@@ -216,7 +216,7 @@ def write_validate_struct_function(writer, struct):
 
     writer.set_is_generated("validator", validate_function)
     writer = writer.function_helper()
-    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 = writer.function(validate_function, "static intptr_t", "uint8_t *message_start, uint8_t *message_end, uint64_t offset")
     scope.variable_def("uint8_t *", "start = message_start + offset")
     scope.variable_def("SPICE_GNUC_UNUSED uint8_t *", "pos")
     scope.variable_def("uint64_t", "mem_size", "nw_size")
@@ -301,7 +301,7 @@ def write_validate_pointer_item(writer, container, item, scope, parent_scope, st
 
         elif target_type.is_struct():
             validate_function = write_validate_struct_function(writer, target_type)
-            writer.assign("ptr_size", "%s(message_start, message_end, %s, minor)" % (validate_function, v))
+            writer.assign("ptr_size", "%s(message_start, message_end, %s)" % (validate_function, v))
             writer.error_check("ptr_size < 0")
 
             if want_extra_size:
@@ -512,12 +512,8 @@ def write_validate_member(writer, mprefix, container, member, parent_scope, star
     if member.has_attr("virtual"):
         return
 
-    if member.has_minor_attr():
-        prefix = "if (minor >= %s)" % (member.get_minor_attr())
-        newline = False
-    else:
-        prefix = ""
-        newline = True
+    prefix = ""
+    newline = True
     item = MemberItemInfo(member, mprefix, container, start)
     with writer.block(prefix, newline=newline, comment=member.name) as scope:
         if member.is_switch():
@@ -527,24 +523,6 @@ def write_validate_member(writer, mprefix, container, member, parent_scope, star
             write_validate_item(writer, container, item, scope, parent_scope, start,
                                 want_nw_size, want_mem_size, want_extra_size)
 
-    if member.has_minor_attr():
-        with writer.block(" else", comment = "minor < %s" % (member.get_minor_attr())):
-            if member.is_array():
-                nelements = "%s__nelements" %(item.prefix)
-                writer.assign(nelements, 0)
-            if want_nw_size:
-                writer.assign(item.nw_size(), 0)
-
-            if want_mem_size:
-                if member.is_fixed_sizeof():
-                    writer.assign(item.mem_size(), member.sizeof())
-                elif member.is_array():
-                    writer.assign(item.mem_size(), 0)
-                else:
-                    raise NotImplementedError("TODO minor check for non-constant items")
-
-            assert not want_extra_size
-
 def write_validate_container(writer, prefix, container, start, parent_scope, want_nw_size, want_mem_size, want_extra_size):
     def prefix_m(prefix, m):
         name = m.name
@@ -782,7 +760,7 @@ def write_parse_ptr_function(writer, target_type):
     writer.set_is_generated("parser", parse_function)
 
     writer = writer.function_helper()
-    scope = writer.function(parse_function, "static uint8_t *", "uint8_t *message_start, SPICE_GNUC_UNUSED uint8_t *message_end, uint8_t *struct_data, PointerInfo *this_ptr_info, SPICE_GNUC_UNUSED int minor")
+    scope = writer.function(parse_function, "static uint8_t *", "uint8_t *message_start, SPICE_GNUC_UNUSED uint8_t *message_end, uint8_t *struct_data, PointerInfo *this_ptr_info")
     scope.variable_def("uint8_t *", "in = message_start + this_ptr_info->offset")
     scope.variable_def("uint8_t *", "end")
 
@@ -977,24 +955,7 @@ def write_member_parser(writer, container, member, dest, scope):
 def write_container_parser(writer, container, dest):
     with dest.declare(writer) as scope:
         for m in container.members:
-            if m.has_minor_attr():
-                writer.begin_block("if (minor >= %s)" % m.get_minor_attr())
             write_member_parser(writer, container, m, dest, scope)
-            if m.has_minor_attr():
-                # We need to zero out the fixed part of all optional fields
-                if not m.member_type.is_array():
-                    writer.end_block(newline=False)
-                    writer.begin_block(" else")
-                    # TODO: This is not right for fields that don't exist in the struct
-                    if m.has_attr("zero"):
-                        pass
-                    elif m.member_type.is_primitive():
-                        writer.assign(dest.get_ref(m.name), "0")
-                    elif m.is_fixed_sizeof():
-                        writer.statement("memset ((char *)&%s, 0, %s)" % (dest.get_ref(m.name), m.sizeof()))
-                    else:
-                        raise NotImplementedError("TODO Clear optional dynamic fields")
-                writer.end_block()
 
 def write_ptr_info_check(writer):
     writer.newline()
@@ -1009,7 +970,7 @@ def write_ptr_info_check(writer):
                 writer.comment("Align to 32 bit").newline()
                 writer.assign("end", "(uint8_t *)SPICE_ALIGN((size_t)end, 4)")
                 writer.assign("*%s" % dest, "(void *)end")
-                writer.assign("end", "%s(message_start, message_end, end, &ptr_info[%s], minor)" % (function, index))
+                writer.assign("end", "%s(message_start, message_end, end, &ptr_info[%s])" % (function, index))
                 writer.error_check("end == NULL")
     writer.newline()
 
@@ -1037,7 +998,7 @@ def write_msg_parser(writer, message):
         writer.ifdef(message.attributes["ifdef"][0])
     parent_scope = writer.function(function_name,
                                    "uint8_t *",
-                                   "uint8_t *message_start, uint8_t *message_end, SPICE_GNUC_UNUSED int minor, size_t *size, message_destructor_t *free_message", True)
+                                   "uint8_t *message_start, uint8_t *message_end, size_t *size, message_destructor_t *free_message", True)
     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")
@@ -1160,7 +1121,7 @@ def write_channel_parser(writer, channel, server):
     for r in ranges:
         d = d + 1
         with writer.if_block("message_type >= %d && message_type < %d" % (r[0], r[1]), d > 1, False):
-            writer.statement("return funcs%d[message_type-%d](message_start, message_end, minor, size_out, free_message)" % (d, r[0]))
+            writer.statement("return funcs%d[message_type-%d](message_start, message_end, size_out, free_message)" % (d, r[0]))
     writer.newline()
 
     writer.statement("return NULL")
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index f39f044..c548a28 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -15,46 +15,6 @@ def lookup_type(name):
 def get_named_types():
     return _types
 
-class FixedSize:
-    def __init__(self, val = 0, minor = 0):
-        if isinstance(val, FixedSize):
-            self.vals = val.vals
-        else:
-            self.vals = [0] * (minor + 1)
-            self.vals[minor] = val
-
-    def __add__(self, other):
-        if isinstance(other, int):
-            other = FixedSize(other)
-
-        new = FixedSize()
-        l = max(len(self.vals), len(other.vals))
-        shared = min(len(self.vals), len(other.vals))
-
-        new.vals = [0] * l
-
-        for i in range(shared):
-            new.vals[i] = self.vals[i] + other.vals[i]
-
-        for i in range(shared,len(self.vals)):
-            new.vals[i] = self.vals[i]
-
-        for i in range(shared,len(other.vals)):
-            new.vals[i] = new.vals[i] + other.vals[i]
-
-        return new
-
-    def __radd__(self, other):
-        return self.__add__(other)
-
-    def __str__(self):
-        s = "%d" % (self.vals[0])
-
-        for i in range(1,len(self.vals)):
-            if self.vals[i] > 0:
-                s = s + " + ((minor >= %d)?%d:0)" % (i, self.vals[i])
-        return s
-
 # Some attribute are propagated from member to the type as they really
 # are part of the type definition, rather than the member. This applies
 # only to attributes that affect pointer or array attributes, as these
@@ -107,8 +67,6 @@ valid_attributes=set([
     # 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',
     # this attribute does not exist on the network, fill just structure with the value
     'virtual',
 ])
@@ -119,7 +77,6 @@ attributes_with_arguments=set([
     'as_ptr',
     'outvar',
     'ifdef',
-    'minor',
     'virtual',
 ])
 
@@ -587,15 +544,9 @@ class Containee:
             raise Exception('attribute %s not expected' % name)
         return name in self.attributes
 
-    def has_minor_attr(self):
-        return self.has_attr("minor")
-
     def has_end_attr(self):
         return self.has_attr("end")
 
-    def get_minor_attr(self):
-        return self.attributes["minor"][0]
-
 class Member(Containee):
     def __init__(self, name, member_type, attribute_list):
         Containee.__init__(self)
@@ -635,9 +586,6 @@ class Member(Containee):
         if self.has_attr("virtual"):
             return 0
         size = self.member_type.get_fixed_nw_size()
-        if self.has_minor_attr():
-            minor = self.get_minor_attr()
-            size = FixedSize(size, minor)
         return size
 
     def contains_extra_size(self):
-- 
2.17.1



More information about the Spice-devel mailing list