Re: [PATCH 1/5] xen: Remove redundant __attribute__((packed)) statements

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 13/03/14 08:06, Jan Beulich wrote:
>>>> On 12.03.14 at 20:08, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/drivers/char/ehci-dbgp.c
>> +++ b/xen/drivers/char/ehci-dbgp.c
>> @@ -257,7 +257,7 @@ struct usb_ctrlrequest {
>>      __le16 wValue;
>>      __le16 wIndex;
>>      __le16 wLength;
>> -} __attribute__ ((packed));
>> +};
>>  
>>  /* USB_DT_DEBUG: for special highspeed devices, replacing serial console */
>>  
>> @@ -269,7 +269,7 @@ struct usb_debug_descriptor {
>>      /* bulk endpoints with 8 byte maxpacket */
>>      u8 bDebugInEndpoint;
>>      u8 bDebugOutEndpoint;
>> -} __attribute__((packed));
>> +};
> While correct, I'm not sure we want to drop these ...

Ok

>
>> --- a/xen/include/asm-x86/edd.h
>> +++ b/xen/include/asm-x86/edd.h
>> @@ -55,27 +55,27 @@ struct edd_info {
>>                  u16 base_address;
>>                  u16 reserved1;
>>                  u32 reserved2;
>> -            } __attribute__ ((packed)) isa;
>> +            } isa;
>>              struct {
>>                  u8 bus;
>>                  u8 slot;
>>                  u8 function;
>>                  u8 channel;
>>                  u32 reserved;
>> -            } __attribute__ ((packed)) pci;
>> +            } pci;
>>              /* pcix is same as pci */
>>              struct {
>>                  u64 reserved;
>> -            } __attribute__ ((packed)) ibnd;
>> +            } ibnd;
>>              struct {
>>                  u64 reserved;
>> -            } __attribute__ ((packed)) xprs;
>> +            } xprs;
>>              struct {
>>                  u64 reserved;
>> -            } __attribute__ ((packed)) htpt;
>> +            } htpt;
>>              struct {
>>                  u64 reserved;
>> -            } __attribute__ ((packed)) unknown;
>> +            } unknown;
>>          } interface_path;
>>          union {
>>              struct {
>> @@ -84,7 +84,7 @@ struct edd_info {
>>                  u16 reserved2;
>>                  u32 reserved3;
>>                  u64 reserved4;
>> -            } __attribute__ ((packed)) ata;
>> +            } ata;
>>              struct {
>>                  u8 device;
>>                  u8 lun;
>> @@ -92,7 +92,7 @@ struct edd_info {
>>                  u8 reserved2;
>>                  u32 reserved3;
>>                  u64 reserved4;
>> -            } __attribute__ ((packed)) atapi;
>> +            } atapi;
>>              struct {
>>                  u16 id;
>>                  u64 lun;
>> @@ -102,35 +102,35 @@ struct edd_info {
>>              struct {
>>                  u64 serial_number;
>>                  u64 reserved;
>> -            } __attribute__ ((packed)) usb;
>> +            } usb;
>>              struct {
>>                  u64 eui;
>>                  u64 reserved;
>> -            } __attribute__ ((packed)) i1394;
>> +            } i1394;
>>              struct {
>>                  u64 wwid;
>>                  u64 lun;
>> -            } __attribute__ ((packed)) fibre;
>> +            } fibre;
>>              struct {
>>                  u64 identity_tag;
>>                  u64 reserved;
>> -            } __attribute__ ((packed)) i2o;
>> +            } i2o;
>>              struct {
>>                  u32 array_number;
>>                  u32 reserved1;
>>                  u64 reserved2;
>> -            } __attribute__ ((packed)) raid;
>> +            } raid;
>>              struct {
>>                  u8 device;
>>                  u8 reserved1;
>>                  u16 reserved2;
>>                  u32 reserved3;
>>                  u64 reserved4;
>> -            } __attribute__ ((packed)) sata;
>> +            } sata;
>>              struct {
>>                  u64 reserved1;
>>                  u64 reserved2;
>> -            } __attribute__ ((packed)) unknown;
>> +            } unknown;
>>          } device_path;
>>          u8 reserved4;
>>          u8 checksum;
> ... and these - they're serving a documentation purpose.
> Furthermore, in the latter case you (correctly) left the "scsi"
> sub-structure unchanged, resulting in an inconsistency with
> all other sub-structures.
>
> Jan

As a hardware interface, I will leave the all as-were.

~Andrew