[Spice-devel] [PATCH spice-protocol 1/2] Add support for a header without sub list.

[Yaniv Kaul]

On 12/28/2011 07:14 PM, Yonit Halperin wrote:
> Add SpiceDataHeaderNoSub.
> Introduce capability SPICE_COMMON_CAP_HEADER_NO_SUB.
> Introduce SPICE_MSG_LIST: the msg body is SpiceSubMessageList.
>
> The advantage of using a header without sub list is to spare the 4 bytes that were sent
> for a lot of messages without sublist.
> Instead, messages that previously contained sub lists, will be split to two msgs.
> The first one will be SPICE_MSG_LIST, holding the sub list, and the second will be the
> main msg.
> When most of the messages do not contain sub lists, the overhead of the additional 10 bytes
> for the header of SPICE_MSG_LIST is negligible.
> ---
>   spice/enums.h    |    1 +
>   spice/protocol.h |    9 ++++++++-
>   2 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/spice/enums.h b/spice/enums.h
> index a587b00..0314f0b 100644
> --- a/spice/enums.h
> +++ b/spice/enums.h
> @@ -344,6 +344,7 @@ enum {
>       SPICE_MSG_WAIT_FOR_CHANNELS,
>       SPICE_MSG_DISCONNECTING,
>       SPICE_MSG_NOTIFY,
> +    SPICE_MSG_LIST,
>   };
>
>   enum {
> diff --git a/spice/protocol.h b/spice/protocol.h
> index ddfe84b..cbd8295 100644
> --- a/spice/protocol.h
> +++ b/spice/protocol.h
> @@ -37,7 +37,7 @@
>
>   #define SPICE_MAGIC (*(uint32_t*)"REDQ")
>   #define SPICE_VERSION_MAJOR 2
> -#define SPICE_VERSION_MINOR 1
> +#define SPICE_VERSION_MINOR 2

if you are bumping the protocol version, why negotiate the capability?
Isn't it implicit (if I can do 2.2 or above, I will omit the sublist) ?
Conversely, if you do have the capability exchange, why bump the version?

>
>   // Encryption&  Ticketing Parameters
>   #define SPICE_MAX_PASSWORD_LENGTH 60
> @@ -55,6 +55,7 @@ enum {
>       SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION,
>       SPICE_COMMON_CAP_AUTH_SPICE,
>       SPICE_COMMON_CAP_AUTH_SASL,
> +    SPICE_COMMON_CAP_HEADER_NO_SUB,
>   };
>
>   typedef struct SPICE_ATTR_PACKED SpiceLinkMess {
> @@ -89,6 +90,12 @@ typedef struct SPICE_ATTR_PACKED SpiceDataHeader {
>       uint32_t sub_list; //offset to SpiceSubMessageList[]
>   } SpiceDataHeader;
>
> +typedef struct SPICE_ATTR_PACKED SpiceDataHeaderNoSub {
> +    uint64_t serial;

Ah, if you could also get rid of the serial in the same sweep... More 
useless bytes.
At least for some channels, it's completely useless. I suspect some do 
use it, for no real reason, though.
Y.

> +    uint16_t type;
> +    uint32_t size;
> +} SpiceDataHeaderNoSub;
> +
>   typedef struct SPICE_ATTR_PACKED SpiceSubMessage {
>       uint16_t type;
>       uint32_t size;