All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Weil <sw@weilnetz.de>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: qemu-trivial@nongnu.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-trivial] [PATCH] srp: Don't use QEMU_PACKED for single elements of a structured type
Date: Wed, 15 Aug 2012 17:59:26 +0200	[thread overview]
Message-ID: <502BC75E.8060404@weilnetz.de> (raw)
In-Reply-To: <20120815141608.GJ10742@stefanha-thinkpad.localdomain>

Am 15.08.2012 16:16, schrieb Stefan Hajnoczi:
> On Fri, Aug 10, 2012 at 10:03:27PM +0200, Stefan Weil wrote:
>> QEMU_PACKED results in a MinGW compiler warning when it is
>> used for single structure elements:
>>
>> warning: 'gcc_struct' attribute ignored
>>
>> Using QEMU_PACKED for the whole structure avoids the compiler warning
>> without changing the memory layout.
> Quick link for other reviewers:
> http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/Type-Attributes.html#Type-Attributes
>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>   hw/srp.h |    8 ++++----
>>   1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/srp.h b/hw/srp.h
>> index 3009bd5..5e0cad5 100644
>> --- a/hw/srp.h
>> +++ b/hw/srp.h
>> @@ -177,13 +177,13 @@ struct srp_tsk_mgmt {
>>       uint8_t    reserved1[6];
>>       uint64_t   tag;
>>       uint8_t    reserved2[4];
>> -    uint64_t   lun QEMU_PACKED;
>> +    uint64_t   lun;
>>       uint8_t    reserved3[2];
>>       uint8_t    tsk_mgmt_func;
>>       uint8_t    reserved4;
>>       uint64_t   task_tag;
>>       uint8_t    reserved5[8];
>> -};
>> +} QEMU_PACKED;
> Here I actually see a difference for the uint64_t task_tag field.
> Previously it was not packed, now it is packed and because it has 4 *
> uint8_t before it there will be a difference in layout.
>
> Looking at how QEMU accesses srp_tsk_mgmt, I think we're safe because we
> never actually access task_tag?
>
> Ben: Any thoughts on this patch?
>
> Stefan

4 * uint8_t + 4 bytes from the packed lun, so there is no change
for task_tag, it's always on a 8 byte boundary!

Regards,
Stefan



WARNING: multiple messages have this Message-ID (diff)
From: Stefan Weil <sw@weilnetz.de>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] srp: Don't use QEMU_PACKED for single elements of a structured type
Date: Wed, 15 Aug 2012 17:59:26 +0200	[thread overview]
Message-ID: <502BC75E.8060404@weilnetz.de> (raw)
In-Reply-To: <20120815141608.GJ10742@stefanha-thinkpad.localdomain>

Am 15.08.2012 16:16, schrieb Stefan Hajnoczi:
> On Fri, Aug 10, 2012 at 10:03:27PM +0200, Stefan Weil wrote:
>> QEMU_PACKED results in a MinGW compiler warning when it is
>> used for single structure elements:
>>
>> warning: 'gcc_struct' attribute ignored
>>
>> Using QEMU_PACKED for the whole structure avoids the compiler warning
>> without changing the memory layout.
> Quick link for other reviewers:
> http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/Type-Attributes.html#Type-Attributes
>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>   hw/srp.h |    8 ++++----
>>   1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/srp.h b/hw/srp.h
>> index 3009bd5..5e0cad5 100644
>> --- a/hw/srp.h
>> +++ b/hw/srp.h
>> @@ -177,13 +177,13 @@ struct srp_tsk_mgmt {
>>       uint8_t    reserved1[6];
>>       uint64_t   tag;
>>       uint8_t    reserved2[4];
>> -    uint64_t   lun QEMU_PACKED;
>> +    uint64_t   lun;
>>       uint8_t    reserved3[2];
>>       uint8_t    tsk_mgmt_func;
>>       uint8_t    reserved4;
>>       uint64_t   task_tag;
>>       uint8_t    reserved5[8];
>> -};
>> +} QEMU_PACKED;
> Here I actually see a difference for the uint64_t task_tag field.
> Previously it was not packed, now it is packed and because it has 4 *
> uint8_t before it there will be a difference in layout.
>
> Looking at how QEMU accesses srp_tsk_mgmt, I think we're safe because we
> never actually access task_tag?
>
> Ben: Any thoughts on this patch?
>
> Stefan

4 * uint8_t + 4 bytes from the packed lun, so there is no change
for task_tag, it's always on a 8 byte boundary!

Regards,
Stefan

  reply	other threads:[~2012-08-15 15:59 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-10 20:03 [Qemu-trivial] [PATCH] Spelling fixes in comments and documentation Stefan Weil
2012-08-10 20:03 ` [Qemu-devel] " Stefan Weil
2012-08-10 20:03 ` [Qemu-trivial] [PATCH] Fix spelling (licenced -> licensed) in GPL Stefan Weil
2012-08-10 20:03   ` [Qemu-devel] " Stefan Weil
2012-08-16 12:35   ` [Qemu-trivial] " Stefan Weil
2012-08-16 12:35     ` [Qemu-devel] " Stefan Weil
2012-08-16 13:05   ` [Qemu-trivial] " Stefan Hajnoczi
2012-08-16 13:05     ` Stefan Hajnoczi
2012-08-10 20:03 ` [Qemu-trivial] [PATCH] srp: Don't use QEMU_PACKED for single elements of a structured type Stefan Weil
2012-08-10 20:03   ` [Qemu-devel] " Stefan Weil
2012-08-15 14:16   ` [Qemu-trivial] " Stefan Hajnoczi
2012-08-15 14:16     ` [Qemu-devel] " Stefan Hajnoczi
2012-08-15 15:59     ` Stefan Weil [this message]
2012-08-15 15:59       ` Stefan Weil
2012-08-16  6:53       ` Stefan Hajnoczi
2012-08-16  6:53         ` [Qemu-devel] " Stefan Hajnoczi
2012-08-16 12:38 ` [Qemu-trivial] [PATCH] Spelling fixes in comments and documentation Stefan Weil
2012-08-16 12:38   ` [Qemu-devel] " Stefan Weil
2012-08-16 12:47 ` [Qemu-trivial] " Peter Maydell
2012-08-16 12:47   ` Peter Maydell
2012-08-16 13:04 ` [Qemu-trivial] " Stefan Hajnoczi
2012-08-16 13:04   ` [Qemu-devel] " Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=502BC75E.8060404@weilnetz.de \
    --to=sw@weilnetz.de \
    --cc=benh@kernel.crashing.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.