[Spice-devel] [PATCH] protocol: use packed-start/end to pack structures

[Alon Levy]

On Mon, Apr 11, 2011 at 01:18:27PM +0200, Christophe Fergeau wrote:
> spice-protocol comes with 2 headers to handle structure packing,
> but controller_prot.h and foreign_menu_prot were both using their
> own preprocessor definitions to handle packing. It's better to have
> structure packing macros centralized since how it's done varies
> between compilers, so it may need to change over time.
> 

Looks good. But didn't test it. Basically on ms compiler you need
a #pragma push and #pragma pop, hence the need to include start-packed
right before the first packed struct, and after the last, as opposed
to gcc that uses an attribute (that is #defined out in ms compiler). Looks
like just what you did.

ACK

> diff --git a/spice/controller_prot.h b/spice/controller_prot.h
> index 697baa0..44d78e1 100644
> --- a/spice/controller_prot.h
> +++ b/spice/controller_prot.h
> @@ -19,23 +19,19 @@
>  #define _H_CONTROLLER_PROT
>  
>  #include <spice/types.h>
> +#include <spice/start-packed.h>
>  
>  #define CONTROLLER_MAGIC      (*(uint32_t*)"CTRL")
>  #define CONTROLLER_VERSION    1
>  
> -#ifdef __GNUC__
> -#define ATTR_PACKED __attribute__ ((__packed__))
> -#else
> -#define ATTR_PACKED __declspec(align(1))
> -#endif
>  
> -typedef struct ATTR_PACKED ControllerInitHeader {
> +typedef struct SPICE_ATTR_PACKED ControllerInitHeader {
>      uint32_t magic;
>      uint32_t version;
>      uint32_t size;
>  } ControllerInitHeader;
>  
> -typedef struct ATTR_PACKED ControllerInit {
> +typedef struct SPICE_ATTR_PACKED ControllerInit {
>      ControllerInitHeader base;
>      uint64_t credentials;
>      uint32_t flags;
> @@ -45,7 +41,7 @@ enum {
>      CONTROLLER_FLAG_EXCLUSIVE = 1 << 0,
>  };
>  
> -typedef struct ATTR_PACKED ControllerMsg {
> +typedef struct SPICE_ATTR_PACKED ControllerMsg {
>      uint32_t id;
>      uint32_t size;
>  } ControllerMsg;
> @@ -88,12 +84,12 @@ enum {
>      CONTROLLER_AUTO_DISPLAY_RES = 1 << 1,
>  };
>  
> -typedef struct ATTR_PACKED ControllerValue {
> +typedef struct SPICE_ATTR_PACKED ControllerValue {
>      ControllerMsg base;
>      uint32_t value;
>  } ControllerValue;
>  
> -typedef struct ATTR_PACKED ControllerData {
> +typedef struct SPICE_ATTR_PACKED ControllerData {
>      ControllerMsg base;
>      uint8_t data[0];
>  } ControllerData;
> @@ -112,6 +108,6 @@ enum {
>  #define SPICE_MENU_INTERNAL_ID_BASE   0x1300
>  #define SPICE_MENU_INTERNAL_ID_SHIFT  8
>  
> -#undef ATTR_PACKED
> +#include <spice/end-packed.h>
>  
>  #endif
> diff --git a/spice/foreign_menu_prot.h b/spice/foreign_menu_prot.h
> index 8c22461..570fafb 100644
> --- a/spice/foreign_menu_prot.h
> +++ b/spice/foreign_menu_prot.h
> @@ -18,28 +18,24 @@
>  #ifndef _H_FOREIGN_MENU_PROT
>  #define _H_FOREIGN_MENU_PROT
>  
> +#include <spice/start-packed.h>
> +
>  #define FOREIGN_MENU_MAGIC      (*(uint32_t*)"FRGM")
>  #define FOREIGN_MENU_VERSION    1
>  
> -#ifdef __GNUC__
> -#define ATTR_PACKED __attribute__ ((__packed__))
> -#else
> -#define ATTR_PACKED __declspec(align(1))
> -#endif
> -
> -typedef struct ATTR_PACKED FrgMenuInitHeader {
> +typedef struct SPICE_ATTR_PACKED FrgMenuInitHeader {
>      uint32_t magic;
>      uint32_t version;
>      uint32_t size;
>  } FrgMenuInitHeader;
>  
> -typedef struct ATTR_PACKED FrgMenuInit {
> +typedef struct SPICE_ATTR_PACKED FrgMenuInit {
>      FrgMenuInitHeader base;
>      uint64_t credentials;
>      uint8_t title[0]; //UTF8
>  } FrgMenuInit;
>  
> -typedef struct ATTR_PACKED FrgMenuMsg {
> +typedef struct SPICE_ATTR_PACKED FrgMenuMsg {
>      uint32_t id;
>      uint32_t size;
>  } FrgMenuMsg;
> @@ -58,7 +54,7 @@ enum {
>      FOREIGN_MENU_APP_DEACTIVATED,
>  };
>  
> -typedef struct ATTR_PACKED FrgMenuSetTitle {
> +typedef struct SPICE_ATTR_PACKED FrgMenuSetTitle {
>      FrgMenuMsg base;
>      uint8_t string[0]; //UTF8
>  } FrgMenuSetTitle;
> @@ -71,7 +67,7 @@ enum {
>  
>  #define FOREIGN_MENU_INVALID_ID 0
>  
> -typedef struct ATTR_PACKED FrgMenuAddItem {
> +typedef struct SPICE_ATTR_PACKED FrgMenuAddItem {
>      FrgMenuMsg base;
>      uint32_t id;
>      uint32_t type;
> @@ -79,7 +75,7 @@ typedef struct ATTR_PACKED FrgMenuAddItem {
>      uint8_t string[0]; //UTF8
>  } FrgMenuAddItem, FrgMenuModItem;
>  
> -typedef struct ATTR_PACKED FrgMenuRmItem {
> +typedef struct SPICE_ATTR_PACKED FrgMenuRmItem {
>      FrgMenuMsg base;
>      uint32_t id;
>  } FrgMenuRmItem;
> @@ -93,7 +89,7 @@ enum {
>      FOREIGN_MENU_EVENT_UNCHECKED
>  };
>  
> -typedef struct ATTR_PACKED FrgMenuEvent {
> +typedef struct SPICE_ATTR_PACKED FrgMenuEvent {
>      FrgMenuMsg base;
>      uint32_t id;
>      uint32_t action; //FOREIGN_MENU_EVENT_?
> @@ -102,6 +98,6 @@ typedef struct ATTR_PACKED FrgMenuEvent {
>  typedef struct FrgMenuMsg FrgMenuActivate;
>  typedef struct FrgMenuMsg FrgMenuDeactivate;
>  
> -#undef ATTR_PACKED
> +#include <spice/end-packed.h>
>  
>  #endif
> -- 
> 1.7.4.2
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel