On Tue, 2018-10-09 at 17:24 +0100, Frediano Ziglio wrote:
> 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.
What do you mean by "external function" here? Can you be more explicit?
One more comment below.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> 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
>
This hunk seems to belong to the previous patch?
> -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):
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel