[Spice-commits] 6 commits - client/red_channel.cpp client/red_client.cpp client/tunnel_channel.cpp common/messages.h python_modules/demarshal.py python_modules/marshal.py python_modules/ptypes.py python_modules/spice_parser.py server/reds.c server/red_tunnel_worker.c spice1.proto spice.proto

Alexander Larsson alexl at kemper.freedesktop.org
Mon Jul 19 07:29:00 PDT 2010


 client/red_channel.cpp         |   22 +++++----
 client/red_client.cpp          |   10 +---
 client/tunnel_channel.cpp      |   19 +++-----
 common/messages.h              |   23 +++++-----
 python_modules/demarshal.py    |   71 ++++++++++++++++++++++---------
 python_modules/marshal.py      |   91 +++++++++++++++++------------------------
 python_modules/ptypes.py       |   35 ++-------------
 python_modules/spice_parser.py |    2 
 server/red_tunnel_worker.c     |   11 ++--
 server/reds.c                  |   10 +---
 spice.proto                    |   26 +++++------
 spice1.proto                   |   16 +++----
 12 files changed, 158 insertions(+), 178 deletions(-)

New commits:
commit e646529d2e72ccd5b25f88adcf2c37f15c601408
Author: Alexander Larsson <alexl at redhat.com>
Date:   Mon Jul 19 16:27:42 2010 +0200

    Don't send CursorHeader if cursor_flags is NONE

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index 4aba7b0..f259ead 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -708,7 +708,10 @@ def write_switch_parser(writer, container, switch, dest, scope):
             if switch.has_end_attr():
                 dest2 = dest.child_at_end(writer, m.member_type)
             elif switch.has_attr("anon"):
-                dest2 = dest
+                if t.is_struct() and not m.has_attr("to_ptr"):
+                    dest2 = dest.child_sub(m.name)
+                else:
+                    dest2 = dest
             else:
                 if t.is_struct():
                     dest2 = dest.child_sub(switch.name + "." + m.name)
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 59ed897..0ae57ec 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -591,9 +591,9 @@ class SwitchCase:
             if v == None:
                 return "1"
             elif var_type.is_enum():
-                checks.append("%s == %s" % (var_cname, var_type.c_enumname_by_name(v)))
+                checks.append("%s == %s" % (var_cname, var_type.c_enumname_by_name(v[1])))
             else:
-                checks.append("(%s & %s)" % (var_cname, var_type.c_enumname_by_name(v)))
+                checks.append("%s(%s & %s)" % (v[0], var_cname, var_type.c_enumname_by_name(v[1])))
         return " || ".join(checks)
 
     def resolve(self, container):
diff --git a/python_modules/spice_parser.py b/python_modules/spice_parser.py
index ac2da8d..43e930c 100644
--- a/python_modules/spice_parser.py
+++ b/python_modules/spice_parser.py
@@ -93,7 +93,7 @@ def SPICE_BNF():
         variableDef = Group(typeSpec + Optional("*", default=None) + identifier + Optional(arraySizeSpec, default=None) + attributes - semi) \
             .setParseAction(parseVariableDef)
 
-        switchCase = Group(Group(OneOrMore(default_.setParseAction(replaceWith(None)) + colon | case_.suppress() + identifier + colon)) + variableDef) \
+        switchCase = Group(Group(OneOrMore(default_.setParseAction(replaceWith(None)) + colon | Group(case_.suppress() + Optional("!", default="") + identifier) + colon)) + variableDef) \
             .setParseAction(lambda toks: ptypes.SwitchCase(toks[0][0], toks[0][1]))
         switchBody = Group(switch_ + lparen + delimitedList(identifier,delim='.', combine=True) + rparen + lbrace + Group(OneOrMore(switchCase)) + rbrace + identifier + attributes - semi) \
             .setParseAction(lambda toks: ptypes.Switch(toks[0][1], toks[0][2], toks[0][3], toks[0][4]))
diff --git a/spice.proto b/spice.proto
index f24445b..3f7c30c 100644
--- a/spice.proto
+++ b/spice.proto
@@ -874,7 +874,10 @@ struct CursorHeader {
 
 struct Cursor {
     cursor_flags flags;
-    CursorHeader header;
+    switch (flags) {
+    case !NONE:
+        CursorHeader header;
+    } u @anon;
     uint8 data[] @as_ptr(data_size);
 };
 
commit 38809803807637336209a89822e12bb5f22148ca
Author: Alexander Larsson <alexl at redhat.com>
Date:   Mon Jul 19 15:48:20 2010 +0200

    Send Clip.rects inline rather than using a pointer

diff --git a/spice.proto b/spice.proto
index fbafd82..f24445b 100644
--- a/spice.proto
+++ b/spice.proto
@@ -395,7 +395,7 @@ struct Clip {
     clip_type type;
     switch (type) {
     case RECTS:
-        ClipRects *rects @outvar(cliprects) @marshall @nonnull;
+        ClipRects rects @outvar(cliprects) @to_ptr;
     } u @anon;
 };
 
commit 7690a24acdfba226566773b309bf249627dec784
Author: Alexander Larsson <alexl at redhat.com>
Date:   Mon Jul 19 15:47:40 2010 +0200

    codegen: Allow @to_ptr to make inline structs demarshal as pointers

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index 63f952b..4aba7b0 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -161,6 +161,7 @@ def write_validate_switch_member(writer, container, switch_member, scope, parent
         with writer.if_block(check, not first, False) as if_scope:
             item.type = c.member.member_type
             item.subprefix = item.prefix + "_" + m.name
+            item.member = c.member
 
             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()):
@@ -459,6 +460,8 @@ def write_validate_primitive_item(writer, container, item, scope, parent_scope,
 
 def write_validate_item(writer, container, item, scope, parent_scope, start,
                         want_nw_size, want_mem_size, want_extra_size):
+    if item.member and item.member.has_attr("to_ptr"):
+        want_nw_size = True
     if item.type.is_pointer():
         write_validate_pointer_item(writer, container, item, scope, parent_scope, start,
                                     want_nw_size, want_mem_size, want_extra_size)
@@ -474,6 +477,11 @@ def write_validate_item(writer, container, item, scope, parent_scope, start,
     else:
         writer.todo("Implement validation of %s" % item.type)
 
+    if item.member and item.member.has_attr("to_ptr"):
+        saved_size = "%s__saved_size" % item.member.name
+        writer.add_function_variable("uint32_t", saved_size)
+        writer.assign(saved_size, item.nw_size())
+
 def write_validate_member(writer, container, member, parent_scope, start,
                           want_nw_size, want_mem_size, want_extra_size):
     if member.has_attr("virtual"):
@@ -708,10 +716,12 @@ def write_switch_parser(writer, container, switch, dest, scope):
                     dest2 = dest.child_sub(switch.name)
             dest2.reuse_scope = block
 
-            if t.is_struct():
-                write_container_parser(writer, t, dest2)
+            if m.has_attr("to_ptr"):
+                write_parse_to_pointer(writer, t, False, dest2, m.name, block)
             elif t.is_pointer():
                 write_parse_pointer(writer, t, False, dest2, m.name, block)
+            elif t.is_struct():
+                write_container_parser(writer, t, dest2)
             elif t.is_primitive():
                 if m.has_attr("zero"):
                     writer.statement("consume_%s(&in)" % (t.primitive_type()))
@@ -813,9 +823,8 @@ def write_array_parser(writer, member, nelements, array, dest, scope):
                     writer.comment("Align ptr_array element to 4 bytes").newline()
                     writer.assign("end", "(uint8_t *)SPICE_ALIGN((size_t)end, 4)")
 
-def write_parse_pointer(writer, t, at_end, dest, member_name, scope):
-    target_type = t.target_type
-    writer.assign("ptr_info[n_ptr].offset", "consume_%s(&in)" % t.primitive_type())
+def write_parse_pointer_core(writer, target_type, offset, at_end, dest, member_name, scope):
+    writer.assign("ptr_info[n_ptr].offset", offset)
     writer.assign("ptr_info[n_ptr].parse", write_parse_ptr_function(writer, target_type))
     if at_end:
         writer.assign("ptr_info[n_ptr].dest", "(void **)end")
@@ -828,6 +837,15 @@ def write_parse_pointer(writer, t, at_end, dest, member_name, scope):
 
     writer.statement("n_ptr++")
 
+def write_parse_pointer(writer, t, at_end, dest, member_name, scope):
+    write_parse_pointer_core(writer, t.target_type, "consume_%s(&in)" % t.primitive_type(),
+                             at_end, dest, member_name, scope)
+
+def write_parse_to_pointer(writer, t, at_end, dest, member_name, scope):
+    write_parse_pointer_core(writer, t, "in - start",
+                             at_end, dest, member_name, scope)
+    writer.increment("in", "%s__saved_size" % member_name)
+
 def write_member_parser(writer, container, member, dest, scope):
     if member.has_attr("virtual"):
         writer.assign(dest.get_ref(member.name), member.attributes["virtual"][0])
@@ -839,7 +857,9 @@ def write_member_parser(writer, container, member, dest, scope):
 
     t = member.member_type
 
-    if t.is_pointer():
+    if member.has_attr("to_ptr"):
+        write_parse_to_pointer(writer, t, member.has_end_attr(), dest, member.name, scope)
+    elif 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, True)
diff --git a/python_modules/marshal.py b/python_modules/marshal.py
index cf6ad08..6b894c1 100644
--- a/python_modules/marshal.py
+++ b/python_modules/marshal.py
@@ -87,10 +87,16 @@ class SubMarshallingSource(MarshallingSource):
         self.is_helper = False
 
     def get_self_ref(self):
-        return "&%s" % self.parent_src.get_ref(self.name)
+        if self.containee.has_attr("to_ptr"):
+            return "%s" % self.parent_src.get_ref(self.name)
+        else:
+            return "&%s" % self.parent_src.get_ref(self.name)
 
     def get_ref(self, member):
-        return self.parent_src.get_ref(self.name) + "." + member
+        if self.containee.has_attr("to_ptr"):
+            return self.parent_src.get_ref(self.name) + "->" + member
+        else:
+            return self.parent_src.get_ref(self.name) + "." + member
 
 def write_marshal_ptr_function(writer, target_type):
     if target_type.is_array():
diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index e354454..59ed897 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -536,7 +536,7 @@ class Member(Containee):
         return self.member_type.is_fixed_sizeof()
 
     def is_extra_size(self):
-        return self.has_end_attr() or self.member_type.is_extra_size()
+        return self.has_end_attr() or self.has_attr("to_ptr") or self.member_type.is_extra_size()
 
     def is_fixed_nw_size(self):
         if self.has_attr("virtual"):
@@ -562,6 +562,8 @@ class Member(Containee):
         return "%s (%s)" % (str(self.name), str(self.member_type))
 
     def get_num_pointers(self):
+        if self.has_attr("to_ptr"):
+            return 1
         return self.member_type.get_num_pointers()
 
     def get_pointer_names(self, marshalled):
commit 12b08f2c3e07cea617255bedfd97b2eedf0f180f
Author: Alexander Larsson <alexl at redhat.com>
Date:   Mon Jul 19 14:10:16 2010 +0200

    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.

diff --git a/client/red_channel.cpp b/client/red_channel.cpp
index b475927..54caccf 100644
--- a/client/red_channel.cpp
+++ b/client/red_channel.cpp
@@ -729,19 +729,19 @@ void RedChannel::handle_disconnect(RedPeer::InMessage* message)
 void RedChannel::handle_notify(RedPeer::InMessage* message)
 {
     SpiceMsgNotify *notify = (SpiceMsgNotify *)message->data();
-    const char *sevirity;
+    const char *severity;
     const char *visibility;
-    const char *message_str = "";
+    char *message_str = (char *)"";
     const char *message_prefix = "";
 
-    static const char* sevirity_strings[] = {"info", "warn", "error"};
+    static const char* severity_strings[] = {"info", "warn", "error"};
     static const char* visibility_strings[] = {"!", "!!", "!!!"};
 
 
     if (notify->severity > SPICE_NOTIFY_SEVERITY_ERROR) {
         THROW("bad severity");
     }
-    sevirity = sevirity_strings[notify->severity];
+    severity = severity_strings[notify->severity];
 
     if (notify->visibilty > SPICE_NOTIFY_VISIBILITY_HIGH) {
         THROW("bad visibility");
@@ -750,22 +750,24 @@ void RedChannel::handle_notify(RedPeer::InMessage* message)
 
 
     if (notify->message_len) {
-        if ((message->size() - sizeof(*notify) < notify->message_len + 1)) {
+        if ((message->size() - sizeof(*notify) < notify->message_len)) {
             THROW("access violation");
         }
-        message_str = (char *)(notify + 1);
-        if (message_str[notify->message_len] != 0) {
-            THROW("invalid message");
-        }
+        message_str = new char[notify->message_len + 1];
+        memcpy(message_str, notify->message, notify->message_len);
+        message_str[notify->message_len] = 0;
         message_prefix = ": ";
     }
 
 
     LOG_INFO("remote channel %u:%u %s%s #%u%s%s",
              get_type(), get_id(),
-             sevirity, visibility,
+             severity, visibility,
              notify->what,
              message_prefix, message_str);
+    if (notify->message_len) {
+        delete [] message_str;
+    }
 }
 
 void RedChannel::handle_wait_for_channels(RedPeer::InMessage* message)
diff --git a/client/red_client.cpp b/client/red_client.cpp
index 3f4f8bf..9a8078e 100644
--- a/client/red_client.cpp
+++ b/client/red_client.cpp
@@ -223,13 +223,11 @@ void Migrate::start(const SpiceMsgMainMigrationBegin* migrate)
         _sport = old_migrate->sport ? old_migrate->sport : -1;;
         _auth_options = _client.get_host_auth_options();
     } else {
-        _host.assign(((char*)migrate) + migrate->host_offset);
+        _host.assign((char *)migrate->host_data);
         _port = migrate->port ? migrate->port : -1;
         _sport = migrate->sport ? migrate->sport : -1;
         _auth_options.type_flags = RedPeer::HostAuthOptions::HOST_AUTH_OP_PUBKEY;
-        _auth_options.host_pubkey.assign(((uint8_t*)migrate)+ migrate->pub_key_offset,
-                                         ((uint8_t*)migrate)+ migrate->pub_key_offset +
-                                         migrate->pub_key_size);
+        _auth_options.host_pubkey.assign(migrate->pub_key_data, migrate->pub_key_data + migrate->pub_key_size);
     }
 
     _con_ciphers = _client.get_connection_ciphers();
@@ -1008,7 +1006,7 @@ void RedClient::handle_agent_tokens(RedPeer::InMessage* message)
 void RedClient::handle_migrate_switch_host(RedPeer::InMessage* message)
 {
     SpiceMsgMainMigrationSwitchHost* migrate = (SpiceMsgMainMigrationSwitchHost*)message->data();
-    char* host = ((char*)migrate) + migrate->host_offset;
+    char* host = (char *)migrate->host_data;
     char* subject = NULL;
 
     if (host[migrate->host_size - 1] != '\0') {
@@ -1016,7 +1014,7 @@ void RedClient::handle_migrate_switch_host(RedPeer::InMessage* message)
     }
 
     if (migrate->cert_subject_size) {
-        subject = ((char*)migrate)+ migrate->cert_subject_offset;
+        subject = (char *)migrate->cert_subject_data;
         if (subject[migrate->cert_subject_size - 1] != '\0') {
             THROW("cert subject is not a null-terminated string");
         }
diff --git a/client/tunnel_channel.cpp b/client/tunnel_channel.cpp
index 7047d46..5258742 100644
--- a/client/tunnel_channel.cpp
+++ b/client/tunnel_channel.cpp
@@ -287,25 +287,20 @@ void TunnelChannel::send_service(TunnelService& service)
     }
 
     Message* service_msg = new Message(SPICE_MSGC_TUNNEL_SERVICE_ADD);
-    SpiceMsgcTunnelAddPrintService add;
+    SpiceMsgcTunnelAddGenericService add;
     SpiceMarshaller *name_out, *description_out;
-    add.base.id = service.id;
-    add.base.group = service.group;
-    add.base.type = service.type;
-    add.base.port = service.port;
+    add.id = service.id;
+    add.group = service.group;
+    add.type = service.type;
+    add.port = service.port;
 
     if (service.type == SPICE_TUNNEL_SERVICE_TYPE_IPP) {
-        add.ip.type = SPICE_TUNNEL_IP_TYPE_IPv4;
+        add.u.ip.type = SPICE_TUNNEL_IP_TYPE_IPv4;
     }
 
-    _marshallers->msgc_tunnel_service_add(service_msg->marshaller(), &add.base,
+    _marshallers->msgc_tunnel_service_add(service_msg->marshaller(), &add,
                                            &name_out, &description_out);
 
-    if (service.type == SPICE_TUNNEL_SERVICE_TYPE_IPP) {
-        spice_marshaller_add(service_msg->marshaller(), (uint8_t *)&(service.ip.s_addr),
-                             sizeof(SpiceTunnelIPv4));
-    }
-
     spice_marshaller_add(name_out, (uint8_t *)service.name.c_str(), service.name.length() + 1);
     spice_marshaller_add(description_out, (uint8_t *)service.description.c_str(), service.description.length() + 1);
 
diff --git a/common/messages.h b/common/messages.h
index 65541c5..8af38f5 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -57,20 +57,20 @@ typedef struct SpiceMsgMainMultiMediaTime {
 typedef struct SpiceMsgMainMigrationBegin {
     uint16_t port;
     uint16_t sport;
-    uint32_t host_offset;
     uint32_t host_size;
+    uint8_t *host_data;
     uint16_t pub_key_type;
-    uint32_t pub_key_offset;
     uint32_t pub_key_size;
+    uint8_t *pub_key_data;
 } SpiceMsgMainMigrationBegin;
 
 typedef struct SpiceMsgMainMigrationSwitchHost {
     uint16_t port;
     uint16_t sport;
-    uint32_t host_offset;
     uint32_t host_size;
-    uint32_t cert_subject_offset;
+    uint8_t *host_data;
     uint32_t cert_subject_size;
+    uint8_t *cert_subject_data;
 } SpiceMsgMainMigrationSwitchHost;
 
 
@@ -401,13 +401,16 @@ typedef struct SpiceMsgTunnelInit {
     uint32_t max_socket_data_size;
 } SpiceMsgTunnelInit;
 
+typedef uint8_t SpiceTunnelIPv4[4];
+
 typedef struct SpiceMsgTunnelIpInfo {
     uint16_t type;
+    union {
+      SpiceTunnelIPv4 ipv4;
+    } u;
     uint8_t data[0];
 } SpiceMsgTunnelIpInfo;
 
-typedef uint8_t SpiceTunnelIPv4[4];
-
 typedef struct SpiceMsgTunnelServiceIpMap {
     uint32_t service_id;
     SpiceMsgTunnelIpInfo virtual_ip;
@@ -450,13 +453,11 @@ typedef struct SpiceMsgcTunnelAddGenericService {
     uint32_t port;
     uint64_t name;
     uint64_t description;
+    union {
+        SpiceMsgTunnelIpInfo ip;
+    } u;
 } SpiceMsgcTunnelAddGenericService;
 
-typedef struct SpiceMsgcTunnelAddPrintService {
-    SpiceMsgcTunnelAddGenericService base;
-    SpiceMsgTunnelIpInfo ip;
-} SpiceMsgcTunnelAddPrintService;
-
 typedef struct SpiceMsgcTunnelRemoveService {
     uint32_t id;
 } SpiceMsgcTunnelRemoveService;
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/server/red_tunnel_worker.c b/server/red_tunnel_worker.c
index fea4546..6092a76 100644
--- a/server/red_tunnel_worker.c
+++ b/server/red_tunnel_worker.c
@@ -1123,21 +1123,21 @@ static void tunnel_worker_free_print_service(TunnelWorker *worker, TunnelPrintSe
 }
 
 static TunnelPrintService *tunnel_worker_add_print_service(TunnelWorker *worker,
-                                                           SpiceMsgcTunnelAddPrintService *redc_service)
+                                                           SpiceMsgcTunnelAddGenericService *redc_service)
 {
     TunnelPrintService *service;
 
     service = (TunnelPrintService *)tunnel_worker_add_service(worker, sizeof(TunnelPrintService),
-                                                              &redc_service->base);
+                                                              redc_service);
 
     if (!service) {
         return NULL;
     }
 
-    if (redc_service->ip.type == SPICE_TUNNEL_IP_TYPE_IPv4) {
-        memcpy(service->ip, redc_service->ip.data, sizeof(SpiceTunnelIPv4));
+    if (redc_service->type == SPICE_TUNNEL_IP_TYPE_IPv4) {
+        memcpy(service->ip, redc_service->u.ip.data, sizeof(SpiceTunnelIPv4));
     } else {
-        red_printf("unexpected ip type=%d", redc_service->ip.type);
+        red_printf("unexpected ip type=%d", redc_service->type);
         tunnel_worker_free_print_service(worker, service);
         return NULL;
     }
@@ -1154,7 +1154,6 @@ static int tunnel_channel_handle_service_add(TunnelChannel *channel,
     TunnelService *out_service = NULL;
     if (service_msg->type == SPICE_TUNNEL_SERVICE_TYPE_IPP) {
         out_service = &tunnel_worker_add_print_service(channel->worker,
-                                                       (SpiceMsgcTunnelAddPrintService *)
                                                        service_msg)->base;
     } else if (service_msg->type == SPICE_TUNNEL_SERVICE_TYPE_GENERIC) {
         out_service = tunnel_worker_add_service(channel->worker, sizeof(TunnelService),
diff --git a/server/reds.c b/server/reds.c
index 97a47ae..cf2d6ab 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3189,22 +3189,18 @@ static void reds_mig_continue(void)
     RedsMigSpice *s = reds->mig_spice;
     SpiceMsgMainMigrationBegin migrate;
     RedsOutItem *item;
-    int host_len;
 
     red_printf("");
-    host_len = strlen(s->host) + 1;
     item = new_out_item(SPICE_MSG_MAIN_MIGRATE_BEGIN);
 
     migrate.port = s->port;
     migrate.sport = s->sport;
-    migrate.host_offset = sizeof(SpiceMsgMainMigrationBegin);
-    migrate.host_size = host_len;
+    migrate.host_size = strlen(s->host) + 1;
+    migrate.host_data = (uint8_t *)s->host;
     migrate.pub_key_type = s->cert_pub_key_type;
-    migrate.pub_key_offset = sizeof(SpiceMsgMainMigrationBegin) + host_len;
     migrate.pub_key_size = s->cert_pub_key_len;
+    migrate.pub_key_data = s->cert_pub_key;
     spice_marshall_msg_main_migrate_begin(item->m, &migrate);
-    spice_marshaller_add(item->m, (uint8_t *)s->host, host_len);
-    spice_marshaller_add(item->m, s->cert_pub_key, s->cert_pub_key_len);
 
     reds_push_pipe_item(item);
 
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:
commit 2962bdaea056d7069303badc3d73ed9530fdbb15
Author: Alexander Larsson <alexl at redhat.com>
Date:   Mon Jul 19 10:12:41 2010 +0200

    codegen: Pass member to SubMarshallingSource rather than name
    
    This way we can check attributes on the member.

diff --git a/python_modules/marshal.py b/python_modules/marshal.py
index df0c3b3..6b2d428 100644
--- a/python_modules/marshal.py
+++ b/python_modules/marshal.py
@@ -29,8 +29,8 @@ class MarshallingSource:
     def child_at_end(self, t):
         return RootMarshallingSource(self, t.c_type(), t.sizeof())
 
-    def child_sub(self, member):
-        return SubMarshallingSource(self, member)
+    def child_sub(self, containee):
+        return SubMarshallingSource(self, containee)
 
     def declare(self, writer):
         return writer.optional_block(self.reuse_scope)
@@ -84,18 +84,19 @@ class RootMarshallingSource(MarshallingSource):
             return writer.partial_block(scope)
 
 class SubMarshallingSource(MarshallingSource):
-    def __init__(self, parent_src, member):
+    def __init__(self, parent_src, containee):
         self.reuse_scope = None
         self.parent_src = parent_src
         self.base_var = parent_src.base_var
-        self.member = member
+        self.containee = containee
+        self.name = containee.name
         self.is_helper = False
 
     def get_self_ref(self):
-        return "&%s" % self.parent_src.get_ref(self.member)
+        return "&%s" % self.parent_src.get_ref(self.name)
 
     def get_ref(self, member):
-        return self.parent_src.get_ref(self.member) + "." + member
+        return self.parent_src.get_ref(self.name) + "." + member
 
 def write_marshal_ptr_function(writer, target_type):
     if target_type.is_array():
@@ -263,9 +264,9 @@ def write_switch_marshaller(writer, container, switch, src, scope):
                 src2 = src
             else:
                 if t.is_struct():
-                    src2 = src.child_sub(switch.name + "." + m.name)
+                    src2 = src.child_sub(switch).child_sub(m)
                 else:
-                    src2 = src.child_sub(switch.name)
+                    src2 = src.child_sub(switch)
             src2.reuse_scope = block
 
             if t.is_struct():
@@ -331,7 +332,7 @@ def write_member_marshaller(writer, container, member, src, scope):
         if member.has_end_attr():
             src2 = src.child_at_end(t)
         else:
-            src2 = src.child_sub(member.name)
+            src2 = src.child_sub(member)
         writer.comment(member.name)
         write_container_marshaller(writer, t, src2)
     else:
commit b8832e039153c5be516c6fc4fca5b0e11b8fc55f
Author: Alexander Larsson <alexl at redhat.com>
Date:   Mon Jul 19 09:45:45 2010 +0200

    codegen: Remove unused methos has_pointer()

diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
index 5e18aa3..e354454 100644
--- a/python_modules/ptypes.py
+++ b/python_modules/ptypes.py
@@ -130,9 +130,6 @@ class Type:
         _types.append(self)
         _types_by_name[self.name] = self
 
-    def has_pointer(self):
-        return False
-
     def has_attr(self, name):
         return self.attributes.has_key(name)
 
@@ -223,9 +220,6 @@ class TypeAlias(Type):
             return self.attributes["ctype"][0]
         return self.name
 
-    def has_pointer(self):
-        return self.the_type.has_pointer()
-
 class EnumBaseType(Type):
     def is_enum(self):
         return isinstance(self, EnumType)
@@ -474,9 +468,6 @@ class PointerType(Type):
         else:
             return "uint64_t"
 
-    def has_pointer(self):
-        return True
-
     def contains_extra_size(self):
         return True
 
@@ -570,9 +561,6 @@ class Member(Containee):
     def __repr__(self):
         return "%s (%s)" % (str(self.name), str(self.member_type))
 
-    def has_pointer(self):
-        return self.member_type.has_pointer()
-
     def get_num_pointers(self):
         return self.member_type.get_num_pointers()
 
@@ -611,9 +599,6 @@ class SwitchCase:
         self.member = self.member.resolve(self)
         return self
 
-    def has_pointer(self):
-        return self.member.has_pointer()
-
     def get_num_pointers(self):
         return self.member.get_num_pointers()
 
@@ -706,12 +691,6 @@ class Switch(Containee):
     def contains_member(self, member):
         return False # TODO: Don't support switch deep member lookup yet
 
-    def has_pointer(self):
-        for c in self.cases:
-            if c.has_pointer():
-                return True
-        return False
-
     def get_num_pointers(self):
         count = 0
         for c in self.cases:
@@ -785,12 +764,6 @@ class ContainerType(Type):
             names = names + m.get_pointer_names(marshalled)
         return names
 
-    def has_pointer(self):
-        for m in self.members:
-            if m.has_pointer():
-                return True
-        return False
-
     def get_nw_offset(self, member, prefix = "", postfix = ""):
         fixed = self.get_fixed_nw_offset(member)
         v = []


More information about the Spice-commits mailing list