From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33901) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1ux5-0001BT-Cb for qemu-devel@nongnu.org; Thu, 16 Aug 2012 03:57:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T1ux4-0000Iz-5k for qemu-devel@nongnu.org; Thu, 16 Aug 2012 03:57:11 -0400 Date: Thu, 16 Aug 2012 07:53:52 +0100 From: Stefan Hajnoczi Message-ID: <20120816065352.GA17802@stefanha-thinkpad.localdomain> References: <1344629007-23311-1-git-send-email-sw@weilnetz.de> <1344629007-23311-3-git-send-email-sw@weilnetz.de> <20120815141608.GJ10742@stefanha-thinkpad.localdomain> <502BC75E.8060404@weilnetz.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <502BC75E.8060404@weilnetz.de> Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] srp: Don't use QEMU_PACKED for single elements of a structured type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org On Wed, Aug 15, 2012 at 05:59:26PM +0200, Stefan Weil wrote: > 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 > >>--- > >> 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! Ah, yes, I see it now! Glad we're switching to struct-level packing :). Stefan