From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 1/5] xen: Remove redundant
	__attribute__((packed)) statements
Date: Thu, 13 Mar 2014 10:38:05 +0000
Message-ID: <53218A8D.6090908@citrix.com>
References: <1394651319-8893-1-git-send-email-andrew.cooper3@citrix.com>
	<1394651319-8893-2-git-send-email-andrew.cooper3@citrix.com>
	<53217504020000780012383E@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <53217504020000780012383E@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>, Keir Fraser <keir@xen.org>, EddieDong <eddie.dong@intel.com>, Jun Nakajima <jun.nakajima@intel.com>, Xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

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