From d9629ca4e700cb35b77579bde4ab78ad7adbb30a Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 19 Jul 2010 14:10:16 +0200 Subject: [PATCH] codegen: Various cleanups Remove all uses of @end in the marshaller, instead just using the C struct array-at-end-of-struct. To make this work we also remove all use of @end for switches (making them C unions). We drop the zero member of the notify message so that we can avoid this use of @end for a primitive in the marshaller (plus its useless to send over the wire). We change the offsets and stuff in the migration messages to real pointers. --- python_modules/demarshal.py | 34 ++++++++++++------- python_modules/marshal.py | 68 +++++++++++++------------------------ spice.proto | 19 ++++------- spice1.proto | 16 ++++----- 4 files changed, 59 insertions(+), 78 deletions(-) diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py index 96aa146..63f952b 100644 --- a/python_modules/demarshal.py +++ b/python_modules/demarshal.py @@ -646,8 +646,11 @@ class SubDemarshallingDestination(DemarshallingDestination): return self.parent_dest.get_ref(self.member) + "." + member # Note: during parsing, byte_size types have been converted to count during validation -def read_array_len(writer, prefix, array, dest, scope): - nelements = "%s__nelements" % prefix +def read_array_len(writer, prefix, array, dest, scope, is_ptr): + if is_ptr: + nelements = "%s__array__nelements" % prefix + else: + nelements = "%s__nelements" % prefix if dest.is_toplevel(): return nelements # Already there for toplevel, need not recalculate element_type = array.element_type @@ -716,8 +719,8 @@ def write_switch_parser(writer, container, switch, dest, scope): writer.assign(dest2.get_ref(m.name), "consume_%s(&in)" % (t.primitive_type())) #TODO validate e.g. flags and enums elif t.is_array(): - nelements = read_array_len(writer, m.name, t, dest, block) - write_array_parser(writer, nelements, t, dest, block) + nelements = read_array_len(writer, m.name, t, dest, block, False) + write_array_parser(writer, m, nelements, t, dest2, block) else: writer.todo("Can't handle type %s" % m.member_type) @@ -759,7 +762,7 @@ def write_parse_ptr_function(writer, target_type): dest.is_helper = True dest.reuse_scope = scope if target_type.is_array(): - write_array_parser(writer, "this_ptr_info->nelements", target_type, dest, scope) + write_array_parser(writer, None, "this_ptr_info->nelements", target_type, dest, scope) else: write_container_parser(writer, target_type, dest) @@ -777,14 +780,17 @@ def write_parse_ptr_function(writer, target_type): return parse_function -def write_array_parser(writer, nelements, array, dest, scope): +def write_array_parser(writer, member, nelements, array, dest, scope): is_byte_size = array.is_bytes_length() element_type = array.element_type if element_type == ptypes.uint8 or element_type == ptypes.int8: - writer.statement("memcpy(end, in, %s)" % (nelements)) + if not member or member.has_attr("end"): + writer.statement("memcpy(end, in, %s)" % (nelements)) + writer.increment("end", nelements) + else: + writer.statement("memcpy(%s, in, %s)" % (dest.get_ref(member.name), nelements)) writer.increment("in", nelements) - writer.increment("end", nelements) else: if array.has_attr("ptr_array"): scope.variable_def("void **", "ptr_array") @@ -817,7 +823,7 @@ def write_parse_pointer(writer, t, at_end, dest, member_name, scope): else: writer.assign("ptr_info[n_ptr].dest", "(void **)&%s" % dest.get_ref(member_name)) if target_type.is_array(): - nelements = read_array_len(writer, member_name, target_type, dest, scope) + nelements = read_array_len(writer, member_name, target_type, dest, scope, True) writer.assign("ptr_info[n_ptr].nelements", nelements) writer.statement("n_ptr++") @@ -836,7 +842,7 @@ def write_member_parser(writer, container, member, dest, scope): if t.is_pointer(): if member.has_attr("chunk"): assert(t.target_type.is_array()) - nelements = read_array_len(writer, member.name, t.target_type, dest, scope) + nelements = read_array_len(writer, member.name, t.target_type, dest, scope, True) writer.comment("Reuse data from network message as chunk").newline() scope.variable_def("SpiceChunks *", "chunks"); writer.assign("chunks", "(SpiceChunks *)end") @@ -866,7 +872,7 @@ def write_member_parser(writer, container, member, dest, scope): writer.assign(dest_var, "consume_%s(&in)" % (t.primitive_type())) #TODO validate e.g. flags and enums elif t.is_array(): - nelements = read_array_len(writer, member.name, t, dest, scope) + nelements = read_array_len(writer, member.name, t, dest, scope, False) if member.has_attr("chunk") and t.element_type.is_fixed_nw_size() and t.element_type.get_fixed_nw_size() == 1: writer.comment("use array as chunk").newline() @@ -892,7 +898,7 @@ def write_member_parser(writer, container, member, dest, scope): else: writer.increment("in", "%s" % (nelements)) else: - write_array_parser(writer, nelements, t, dest, scope) + write_array_parser(writer, member, nelements, t, dest, scope) elif t.is_struct(): if member.has_end_attr(): dest2 = dest.child_at_end(writer, t) @@ -915,7 +921,9 @@ def write_container_parser(writer, container, dest): 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.member_type.is_primitive(): + 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())) diff --git a/python_modules/marshal.py b/python_modules/marshal.py index 6b2d428..cf6ad08 100644 --- a/python_modules/marshal.py +++ b/python_modules/marshal.py @@ -49,8 +49,8 @@ class RootMarshallingSource(MarshallingSource): self.base_var = "src" self.c_type = c_type self.sizeof = sizeof - self.pointer = pointer # None == at "end" - self.update_end = False + self.pointer = pointer + assert pointer != None def get_self_ref(self): return self.base_var @@ -69,13 +69,7 @@ class RootMarshallingSource(MarshallingSource): if not self.reuse_scope: scope.newline() - if self.pointer: - writer.assign(self.base_var, "(%s *)%s" % (self.c_type, self.pointer)) - if self.update_end: - writer.assign("end", "((uint8_t *)%s) + %s" % (self.base_var, self.sizeof)) - else: - writer.assign(self.base_var, "(%s *)end" % self.c_type) - writer.increment("end", "%s" % self.sizeof) + writer.assign(self.base_var, "(%s *)%s" % (self.c_type, self.pointer)) writer.newline() if self.reuse_scope: @@ -119,18 +113,16 @@ def write_marshal_ptr_function(writer, target_type): writer.header = header writer.out_prefix = "" if target_type.is_array(): - scope = writer.function(marshal_function, "SPICE_GNUC_UNUSED static void *", "SpiceMarshaller *m, %s_t *ptr, int count" % target_type.element_type.primitive_type() + names_args) + scope = writer.function(marshal_function, "SPICE_GNUC_UNUSED static void", "SpiceMarshaller *m, %s_t *ptr, int count" % target_type.element_type.primitive_type() + names_args) else: - scope = writer.function(marshal_function, "void *", "SpiceMarshaller *m, %s *ptr" % target_type.c_type() + names_args) - header.writeln("void *" + marshal_function + "(SpiceMarshaller *m, %s *msg" % target_type.c_type() + names_args + ");") - scope.variable_def("SPICE_GNUC_UNUSED uint8_t *", "end") + scope = writer.function(marshal_function, "void", "SpiceMarshaller *m, %s *ptr" % target_type.c_type() + names_args) + header.writeln("void " + marshal_function + "(SpiceMarshaller *m, %s *msg" % target_type.c_type() + names_args + ");") scope.variable_def("SPICE_GNUC_UNUSED SpiceMarshaller *", "m2") for n in names: writer.assign("*%s_out" % n, "NULL") writer.newline() - writer.assign("end", "(uint8_t *)(ptr+1)") if target_type.is_struct(): src = RootMarshallingSource(None, target_type.c_type(), target_type.sizeof(), "ptr") @@ -143,8 +135,6 @@ def write_marshal_ptr_function(writer, target_type): else: writer.todo("Unsuppored pointer marshaller type") - writer.statement("return end") - writer.end_block() return marshal_function @@ -172,9 +162,9 @@ def get_array_size(array, container_src): elif array.is_bytes_length(): return container_src.get_ref(array.size[2]) else: - raise NotImplementedError("TODO array size type not handled yet") + raise NotImplementedError("TODO array size type not handled yet: %s" % array) -def write_array_marshaller(writer, at_end, member, array, container_src, scope): +def write_array_marshaller(writer, member, array, container_src, scope): element_type = array.element_type if array.is_remaining_length(): @@ -196,8 +186,7 @@ def write_array_marshaller(writer, at_end, member, array, container_src, scope): if array.has_attr("ptr_array"): element = "*" + element - if not at_end: - writer.assign(element_array, container_src.get_ref(member.name)) + writer.assign(element_array, container_src.get_ref(member.name)) if is_byte_size: size_start_var = "%s__size_start" % member.name @@ -206,23 +195,16 @@ def write_array_marshaller(writer, at_end, member, array, container_src, scope): with writer.index() as index: with writer.for_loop(index, nelements) as array_scope: - if at_end: - writer.assign(element, "(%s *)end" % element_type.c_type()) - writer.increment("end", element_type.sizeof()) - if element_type.is_primitive(): writer.statement("spice_marshaller_add_%s(m, *%s)" % (element_type.primitive_type(), element)) elif element_type.is_struct(): src2 = RootMarshallingSource(container_src, element_type.c_type(), element_type.sizeof(), element) - if array.is_extra_size(): - src2.update_end = True src2.reuse_scope = array_scope write_container_marshaller(writer, element_type, src2) else: writer.todo("array element unhandled type").newline() - if not at_end: - writer.statement("%s++" % element_array) + writer.statement("%s++" % element_array) if is_byte_size: size_var = member.container.lookup_member(array.size[1]) @@ -235,12 +217,15 @@ def write_pointer_marshaller(writer, member, src): ptr_func = write_marshal_ptr_function(writer, t.target_type) submarshaller = "spice_marshaller_get_ptr_submarshaller(m, %d)" % (1 if member.get_fixed_nw_size() == 8 else 0) if member.has_attr("marshall"): + rest_args = "" + if t.target_type.is_array(): + rest_args = ", %s" % get_array_size(t.target_type, src) writer.assign("m2", submarshaller) if t.has_attr("nonnull"): - writer.statement("%s(m2, %s)" % (ptr_func, src.get_ref(member.name))) + writer.statement("%s(m2, %s%s)" % (ptr_func, src.get_ref(member.name), rest_args)) else: with writer.if_block("%s != NULL" % src.get_ref(member.name)) as block: - writer.statement("%s(m2, %s)" % (ptr_func, src.get_ref(member.name))) + writer.statement("%s(m2, %s%s)" % (ptr_func, src.get_ref(member.name), rest_args)) else: writer.assign("*%s_out" % (writer.out_prefix + member.name), submarshaller) @@ -258,10 +243,11 @@ def write_switch_marshaller(writer, container, switch, src, scope): writer.out_prefix = "%s_%s" % (m.attributes["outvar"][0], writer.out_prefix) with writer.if_block(check, not first, False) as block: t = m.member_type - if switch.has_end_attr(): - src2 = src.child_at_end(m.member_type) - elif switch.has_attr("anon"): - src2 = src + if switch.has_attr("anon"): + if t.is_struct(): + src2 = src.child_sub(m) + else: + src2 = src else: if t.is_struct(): src2 = src.child_sub(switch).child_sub(m) @@ -280,7 +266,7 @@ def write_switch_marshaller(writer, container, switch, src, scope): writer.statement("spice_marshaller_add_%s(m, %s)" % (t.primitive_type(), src2.get_ref(m.name))) #TODO validate e.g. flags and enums elif t.is_array(): - write_array_marshaller(writer, switch.has_end_attr(), m, t, src, scope) + write_array_marshaller(writer, m, t, src2, scope) else: writer.todo("Can't handle type %s" % m.member_type) @@ -321,18 +307,12 @@ def write_member_marshaller(writer, container, member, src, scope): scope.variable_def("void *", var) writer.statement("%s = spice_marshaller_add_%s(m, %s)" % (var, t.primitive_type(), 0)) - elif member.has_end_attr(): - writer.statement("spice_marshaller_add_%s(m, *(%s_t *)end)" % (t.primitive_type(), t.primitive_type())) - writer.increment("end", t.sizeof()) else: writer.statement("spice_marshaller_add_%s(m, %s)" % (t.primitive_type(), src.get_ref(member.name))) elif t.is_array(): - write_array_marshaller(writer, member.has_end_attr(), member, t, src, scope) + write_array_marshaller(writer, member, t, src, scope) elif t.is_struct(): - if member.has_end_attr(): - src2 = src.child_at_end(t) - else: - src2 = src.child_sub(member) + src2 = src.child_sub(member) writer.comment(member.name) write_container_marshaller(writer, t, src2) else: @@ -364,7 +344,6 @@ def write_message_marshaller(writer, message, is_server, private): scope = writer.function(function_name, "static void" if private else "void", "SpiceMarshaller *m, %s *msg" % message.c_type() + names_args) - scope.variable_def("SPICE_GNUC_UNUSED uint8_t *", "end") scope.variable_def("SPICE_GNUC_UNUSED SpiceMarshaller *", "m2") for n in names: @@ -373,7 +352,6 @@ def write_message_marshaller(writer, message, is_server, private): src = RootMarshallingSource(None, message.c_type(), message.sizeof(), "msg") src.reuse_scope = scope - writer.assign("end", "(uint8_t *)(msg+1)") write_container_marshaller(writer, message, src) writer.end_block() diff --git a/spice.proto b/spice.proto index ca01c12..fbafd82 100644 --- a/spice.proto +++ b/spice.proto @@ -131,7 +131,6 @@ channel BaseChannel { uint32 what; /* error_code/warn_code/info_code */ uint32 message_len; uint8 message[message_len] @end @nomarshal; - uint8 zero @end @ctype(uint8_t) @nomarshal; } notify; client: @@ -166,13 +165,11 @@ channel MainChannel : BaseChannel { message { uint16 port; uint16 sport; - uint32 host_offset; uint32 host_size; + uint8 *host_data[host_size] @zero_terminated @marshall @nonnull; pubkey_type pub_key_type; - uint32 pub_key_offset; uint32 pub_key_size; - uint8 host_data[host_size] @end @ctype(uint8_t) @zero_terminated @nomarshal; - uint8 pub_key_data[pub_key_size] @end @ctype(uint8_t) @zero_terminated @nomarshal; + uint8 *pub_key_data[pub_key_size] @zero_terminated @marshall @nonnull; } @ctype(SpiceMsgMainMigrationBegin) migrate_begin = 101; Empty migrate_cancel; @@ -217,12 +214,10 @@ channel MainChannel : BaseChannel { message { uint16 port; uint16 sport; - uint32 host_offset; uint32 host_size; - uint32 cert_subject_offset; + uint8 *host_data[host_size] @zero_terminated @marshall; uint32 cert_subject_size; - uint8 host_data[host_size] @end @ctype(uint8_t) @zero_terminated; - uint8 cert_subject_data[cert_subject_size] @end @ctype(uint8_t) @zero_terminated; + uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall; } @ctype(SpiceMsgMainMigrationSwitchHost) migrate_switch_host; client: @@ -994,8 +989,8 @@ struct TunnelIpInfo { tunnel_ip_type type; switch (type) { case IPv4: - uint8 ipv4[4] @ctype(uint8_t); - } u @end @nomarshal; + uint8 ipv4[4]; + } u; } @ctype(SpiceMsgTunnelIpInfo); channel TunnelChannel : BaseChannel { @@ -1049,7 +1044,7 @@ channel TunnelChannel : BaseChannel { switch (type) { case IPP: TunnelIpInfo ip @ctype(SpiceMsgTunnelIpInfo); - } u @end; + } u; } @ctype(SpiceMsgcTunnelAddGenericService) service_add = 101; message { diff --git a/spice1.proto b/spice1.proto index b4012bd..e38a214 100644 --- a/spice1.proto +++ b/spice1.proto @@ -166,13 +166,13 @@ channel MainChannel : BaseChannel { message { uint16 port; uint16 sport; - uint32 host_offset; + uint32 host_offset @zero; uint32 host_size; pubkey_type pub_key_type @minor(2); - uint32 pub_key_offset @minor(2); + uint32 pub_key_offset @minor(2) @zero; uint32 pub_key_size @minor(2); - uint8 host_data[host_size] @end @ctype(uint8_t) @zero_terminated @nomarshal; - uint8 pub_key_data[pub_key_size] @minor(2) @end @ctype(uint8_t) @zero_terminated @nomarshal; + uint8 host_data[host_size] @as_ptr @zero_terminated; + uint8 pub_key_data[pub_key_size] @minor(2) @as_ptr @zero_terminated; } @ctype(SpiceMsgMainMigrationBegin) migrate_begin = 101; Empty migrate_cancel; @@ -217,12 +217,12 @@ channel MainChannel : BaseChannel { message { uint16 port; uint16 sport; - uint32 host_offset; + uint32 host_offset @zero; uint32 host_size; - uint32 cert_subject_offset; + uint32 cert_subject_offset @zero; uint32 cert_subject_size; - uint8 host_data[host_size] @end @ctype(uint8_t) @zero_terminated; - uint8 cert_subject_data[cert_subject_size] @end @ctype(uint8_t) @zero_terminated; + uint8 host_data[host_size] @as_ptr @zero_terminated; + uint8 cert_subject_data[cert_subject_size] @as_ptr @zero_terminated; } @ctype(SpiceMsgMainMigrationSwitchHost) migrate_switch_host; client: