[Spice-devel] [PATCH spice-server 3/3] Use start/end-packet.h headers instead of direct GCC attribute

Victor Toso victortoso at redhat.com
Fri Aug 2 09:20:40 UTC 2019


Hi,

On Mon, Jul 22, 2019 at 09:47:05AM -0400, Frediano Ziglio wrote:
> > 
> > > 
> > > Hi,
> > > 
> > > 
> > > Here we can also remove some of the packing
> > > 
> > 
> > Do you mean the nested ones? I'm not sure and I would prefer a follow up,
> > it's more "conservative" to keep the attribute if there was in the previous
> > commit.
> > 
> 
> I tested, nested attributes are needed, otherwise ABI is broken.

Acked-by: Victor Toso <victortoso at redhat.com>
for the series

> 
> > > Otherwise, ack for the series
> > > 
> > > 
> > > On 7/22/19 2:08 PM, Frediano Ziglio wrote:
> > > > All other code use these headers.
> > > >
> > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > > ---
> > > >   server/migration-protocol.h | 32 ++++++++++++++++++--------------
> > > >   server/reds.c               |  4 +++-
> > > >   2 files changed, 21 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/server/migration-protocol.h b/server/migration-protocol.h
> > > > index c8ec56e28..2fc8e0364 100644
> > > > --- a/server/migration-protocol.h
> > > > +++ b/server/migration-protocol.h
> > > > @@ -28,11 +28,13 @@
> > > >    * src-server to dst-server migration data messages
> > > >    * ************************************************/
> > > >   
> > > > +#include <spice/start-packed.h>
> > > > +
> > > >   /* increase the version when the version of any
> > > >    * of the migration data messages is increased */
> > > >   #define SPICE_MIGRATION_PROTOCOL_VERSION 1
> > > >   
> > > > -typedef struct __attribute__ ((__packed__)) SpiceMigrateDataHeader {
> > > > +typedef struct SPICE_ATTR_PACKED SpiceMigrateDataHeader {
> > > >       uint32_t magic;
> > > >       uint32_t version;
> > > >   } SpiceMigrateDataHeader;
> > > > @@ -46,7 +48,7 @@ typedef struct __attribute__ ((__packed__))
> > > > SpiceMigrateDataHeader {
> > > >   #define SPICE_MIGRATE_DATA_CHAR_DEVICE_VERSION 1
> > > >   
> > > >   /* Should be the first field of any of the char_devices migration data
> > > >   (see write_data_ptr) */
> > > > -typedef struct __attribute__ ((__packed__)) SpiceMigrateDataCharDevice {
> > > > +typedef struct SPICE_ATTR_PACKED SpiceMigrateDataCharDevice {
> > > >       uint32_t version;
> > > >       uint8_t connected;
> > > >       uint32_t num_client_tokens;
> > > > @@ -64,7 +66,7 @@ typedef struct __attribute__ ((__packed__))
> > > > SpiceMigrateDataCharDevice {
> > > >   #define SPICE_MIGRATE_DATA_SPICEVMC_VERSION 1 /* NOTE: increase version
> > > >   when CHAR_DEVICE_VERSION
> > > >                                                    is increased */
> > > >   #define SPICE_MIGRATE_DATA_SPICEVMC_MAGIC SPICE_MAGIC_CONST("SVMD")
> > > > -typedef struct __attribute__ ((__packed__)) SpiceMigrateDataSpiceVmc {
> > > > +typedef struct SPICE_ATTR_PACKED SpiceMigrateDataSpiceVmc {
> > > >       SpiceMigrateDataCharDevice base;
> > > >   } SpiceMigrateDataSpiceVmc;
> > > >   
> > > > @@ -75,7 +77,7 @@ typedef struct __attribute__ ((__packed__))
> > > > SpiceMigrateDataSpiceVmc {
> > > >   #define SPICE_MIGRATE_DATA_SMARTCARD_VERSION 1 /* NOTE: increase
> > > >   version
> > > >   when CHAR_DEVICE_VERSION
> > > >                                                     is increased */
> > > >   #define SPICE_MIGRATE_DATA_SMARTCARD_MAGIC SPICE_MAGIC_CONST("SCMD")
> > > > -typedef struct __attribute__ ((__packed__)) SpiceMigrateDataSmartcard {
> > > > +typedef struct SPICE_ATTR_PACKED SpiceMigrateDataSmartcard {
> > > >       SpiceMigrateDataCharDevice base;
> > > >       uint8_t reader_added;
> > > >       uint32_t read_size; /* partial data read from dev */
> > > > @@ -89,11 +91,11 @@ typedef struct __attribute__ ((__packed__))
> > > > SpiceMigrateDataSmartcard {
> > > >                                                is increased */
> > > >   #define SPICE_MIGRATE_DATA_MAIN_MAGIC SPICE_MAGIC_CONST("MNMD")
> > > >   
> > > > -typedef struct __attribute__ ((__packed__)) SpiceMigrateDataMain {
> > > > +typedef struct SPICE_ATTR_PACKED SpiceMigrateDataMain {
> > > >       SpiceMigrateDataCharDevice agent_base;
> > > >       uint8_t client_agent_started; /* for discarding messages */
> > > >   
> > > > -    struct __attribute__ ((__packed__)) {
> > > > +    struct SPICE_ATTR_PACKED {
> > > >           /* partial data read from device. Such data is stored only
> > > >            * if the chunk header or the entire msg header haven't yet
> > > >            been
> > > >            read completely.
> > > >            * Once the headers are read, partial reads of chunks can be
> > > >            sent
> > > >            as
> > > > @@ -107,7 +109,7 @@ typedef struct __attribute__ ((__packed__))
> > > > SpiceMigrateDataMain {
> > > >           uint8_t msg_filter_result;
> > > >       } agent2client;
> > > >   
> > > > -    struct __attribute__ ((__packed__)) {
> > > > +    struct SPICE_ATTR_PACKED {
> > > >           uint32_t msg_remaining;
> > > >           uint8_t msg_filter_result;
> > > >       } client2agent;
> > > > @@ -128,7 +130,7 @@ typedef struct __attribute__ ((__packed__))
> > > > SpiceMigrateDataMain {
> > > >    * */
> > > >   #define MIGRATE_DATA_DISPLAY_MAX_CACHE_CLIENTS 4
> > > >   
> > > > -typedef struct __attribute__ ((__packed__)) SpiceMigrateDataDisplay {
> > > > +typedef struct SPICE_ATTR_PACKED SpiceMigrateDataDisplay {
> > > >       uint64_t message_serial;
> > > >       uint8_t low_bandwidth_setting;
> > > >   
> > > > @@ -159,28 +161,28 @@ typedef struct __attribute__ ((__packed__))
> > > > SpiceMigrateDataDisplay {
> > > >   
> > > >   } SpiceMigrateDataDisplay;
> > > >   
> > > > -typedef struct __attribute__ ((__packed__)) SpiceMigrateDataRect {
> > > > +typedef struct SPICE_ATTR_PACKED SpiceMigrateDataRect {
> > > >       int32_t left;
> > > >       int32_t top;
> > > >       int32_t right;
> > > >       int32_t bottom;
> > > >   } SpiceMigrateDataRect;
> > > >   
> > > > -typedef struct __attribute__ ((__packed__))
> > > > MigrateDisplaySurfaceLossless
> > > > {
> > > > +typedef struct SPICE_ATTR_PACKED MigrateDisplaySurfaceLossless {
> > > >       uint32_t id;
> > > >   } MigrateDisplaySurfaceLossless;
> > > >   
> > > > -typedef struct __attribute__ ((__packed__)) MigrateDisplaySurfaceLossy {
> > > > +typedef struct SPICE_ATTR_PACKED MigrateDisplaySurfaceLossy {
> > > >       uint32_t id;
> > > >       SpiceMigrateDataRect lossy_rect;
> > > >   } MigrateDisplaySurfaceLossy;
> > > >   
> > > > -typedef struct __attribute__ ((__packed__))
> > > > MigrateDisplaySurfacesAtClientLossless {
> > > > +typedef struct SPICE_ATTR_PACKED MigrateDisplaySurfacesAtClientLossless
> > > > {
> > > >       uint32_t num_surfaces;
> > > >       MigrateDisplaySurfaceLossless surfaces[0];
> > > >   } MigrateDisplaySurfacesAtClientLossless;
> > > >   
> > > > -typedef struct __attribute__ ((__packed__))
> > > > MigrateDisplaySurfacesAtClientLossy {
> > > > +typedef struct SPICE_ATTR_PACKED MigrateDisplaySurfacesAtClientLossy {
> > > >       uint32_t num_surfaces;
> > > >       MigrateDisplaySurfaceLossy surfaces[0];
> > > >   } MigrateDisplaySurfacesAtClientLossy;
> > > > @@ -193,10 +195,12 @@ typedef struct __attribute__ ((__packed__))
> > > > MigrateDisplaySurfacesAtClientLossy
> > > >   #define SPICE_MIGRATE_DATA_INPUTS_MAGIC SPICE_MAGIC_CONST("ICMD")
> > > >   
> > > >   
> > > > -typedef struct __attribute__ ((__packed__)) SpiceMigrateDataInputs {
> > > > +typedef struct SPICE_ATTR_PACKED SpiceMigrateDataInputs {
> > > >       uint16_t motion_count;
> > > >   } SpiceMigrateDataInputs;
> > > >   
> > > > +#include <spice/end-packed.h>
> > > > +
> > > >   static inline int
> > > >   migration_protocol_validate_header(SpiceMigrateDataHeader *header,
> > > >                                                        uint32_t magic,
> > > >                                                        uint32_t version)
> > > > diff --git a/server/reds.c b/server/reds.c
> > > > index 78bbe5a0f..37854ace9 100644
> > > > --- a/server/reds.c
> > > > +++ b/server/reds.c
> > > > @@ -256,7 +256,8 @@ struct RedCharDeviceVDIPortPrivate {
> > > >   };
> > > >   
> > > >   /* messages that are addressed to the agent and are created in the
> > > >   server
> > > >   */
> > > > -typedef struct __attribute__ ((__packed__)) VDInternalBuf {
> > > > +#include <spice/start-packed.h>
> > > > +typedef struct SPICE_ATTR_PACKED VDInternalBuf {
> > > >       VDIChunkHeader chunk_header;
> > > >       VDAgentMessage header;
> > > >       union {
> > > > @@ -265,6 +266,7 @@ typedef struct __attribute__ ((__packed__))
> > > > VDInternalBuf {
> > > >       }
> > > >       u;
> > > >   } VDInternalBuf;
> > > > +#include <spice/end-packed.h>
> > > >   
> > > >   SPICE_DECLARE_TYPE(RedCharDeviceVDIPort, red_char_device_vdi_port,
> > > >   CHAR_DEVICE_VDIPORT);
> > > >   #define RED_TYPE_CHAR_DEVICE_VDIPORT
> > > >   red_char_device_vdi_port_get_type()
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190802/40863a72/attachment.sig>


More information about the Spice-devel mailing list