[Spice-devel] [PATCH spice-common 4/7] messages: Remove fields not used by the protocol

[Frediano Ziglio]

> 
> These fields are not used by the protocol.
> Avoid spice-gtk and spice-server to use them by mistake.
> This can cause memory errors (data_size is not used or
> is not set correctly) and useless code (spice-gtk uses
> the pub_key* fields but these fields are not sent to
> the server as the protocol does not have them).
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  common/messages.h | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/common/messages.h b/common/messages.h
> index 3e37235..91ac7ad 100644
> --- a/common/messages.h
> +++ b/common/messages.h
> @@ -43,7 +43,6 @@
>  SPICE_BEGIN_DECLS
>  
>  typedef struct SpiceMsgData {
> -    uint32_t data_size;
>      uint8_t data[0];
>  } SpiceMsgData;
>  
> @@ -75,9 +74,6 @@ typedef struct SpiceMigrationDstInfo {
>      uint16_t sport;
>      uint32_t host_size;
>      uint8_t *host_data;
> -    uint16_t pub_key_type;
> -    uint32_t pub_key_size;
> -    uint8_t *pub_key_data;
>      uint32_t cert_subject_size;
>      uint8_t *cert_subject_data;
>  } SpiceMigrationDstInfo;

A bit more digging on this weird one.

I found this patch:

commit 9cf6d39b369f9c22615fc329e307126721125ecd
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Sun Sep 18 10:31:38 2011 +0300

    server,proto: tell the clients to connect to the migration target before migraton starts
    
    (1) send SPICE_MSG_MAIN_MIGRATE_BEGIN upon spice_server_migrate_connect
        (to all the clients that support it)
    (2) wait for SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECT_ERROR) from all the relevant clients,
        or a timeout, in order to complete client_migrate_info monitor command
    (cherry picked from commit 5560c56ef05c74da5e0e0825dc1f134019593cad branch 0.8;
     Was modified to support the separation of main channel from reds, and multiple clients)
    
    Conflicts:
    
            server/reds.c

diff --git a/common/messages.h b/common/messages.h
index 8151dc0..a54190f 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -66,6 +66,8 @@ typedef struct SpiceMsgMainMigrationBegin {
     uint16_t pub_key_type;
     uint32_t pub_key_size;
     uint8_t *pub_key_data;
+    uint32_t cert_subject_size;
+    uint8_t *cert_subject_data;
 } SpiceMsgMainMigrationBegin;
 
 typedef struct SpiceMsgMainMigrationSwitchHost {
diff --git a/spice.proto b/spice.proto
index abf3ec3..78c1fad 100644
--- a/spice.proto
+++ b/spice.proto
@@ -167,9 +167,8 @@ channel MainChannel : BaseChannel {
        uint16 sport;
        uint32 host_size;
        uint8 *host_data[host_size] @zero_terminated @marshall @nonnull;
-       pubkey_type pub_key_type;
-       uint32 pub_key_size;
-       uint8 *pub_key_data[pub_key_size] @zero_terminated  @marshall @nonnull;
+       uint32 cert_subject_size;
+       uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall;
     } @ctype(SpiceMsgMainMigrationBegin) migrate_begin = 101;
 
     Empty migrate_cancel;


spice-gtk is still reading the old fields.
But as the marshaller code (spice-server) is ignoring them when
demarshalled (spice-gtk) all fields will be zeroes and client
will copy this "empty" array to the "pubkey" property.
I suppose, as nobody noticed it in 8 years that the relative
code in spice-gtk is not used.

Frediano