Re: [Qemu-devel] [PATCH] build fix: xen on NetBSD/amd64

[Christoph Egger <Christoph.Egger@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 3576 bytes --]

On Friday 10 July 2009 17:22:04 malc wrote:
> On Fri, 10 Jul 2009, Christoph Egger wrote:
> > On Friday 10 July 2009 12:18:39 malc wrote:
> > > On Fri, 10 Jul 2009, Christoph Egger wrote:
> > > > On Thursday 09 July 2009 21:21:06 Anthony Liguori wrote:
> > > > > Christoph Egger wrote:
> > > > > > Hi!
> > > > > >
> > > > > > Attached patch fixes this build error on NetBSD/amd64:
> > > > > >
> > > > > > hw/xen_blkif.h:20: warning: #pragma pack(psuh[, id], <n>) is not
> > > > > > supported on this target
> > > > > > hw/xen_blkif.h:36: warning: #pragma pack(pop[, id], <n>) is not
> > > > > > supported on this target
> > > > > >
> > > > > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> > > > >
> > > > > You'll invoke the fury of malc for introducing an identifier that
> > > > > begins with '__' :-)
> > > >
> > > > In NetBSD, there is this in <sys/cdefs.h> :
> > > >
> > > > #if __GNUC_PREREQ__(2, 7)
> > > > #define __packed        __attribute__((__packed__))
> > > > #define __aligned(x)    __attribute__((__aligned__(x)))
> > > > #define __section(x)    __attribute__((__section__(x)))
> > > > #elif defined(__PCC__)
> > > > #define __packed        _Pragma("packed 1")
> > > > #define __aligned(x)    _Pragma("aligned " __STRING(x))
> > > > #define __section(x)    _Pragma("section " ## x)
> > > > #elif defined(__lint__)
> > > > #define __packed        /* delete */
> > > > #define __aligned(x)    /* delete */
> > > > #define __section(x)    /* delete */
> > > > #else
> > > > #define __packed        error: no __packed for this compiler
> > > > #define __aligned(x)    error: no __aligned for this compiler
> > > > #define __section(x)    error: no __section for this compiler
> > > > #endif
> > > >
> > > > > There really isn't pragma pack on NetBSD?  That's ashame.
> > > >
> > > > No. Above defines are sufficient and portable.
> > >
> > > Above defines are sufficient but not anywhere near portable.
> >
> > Well, across different compilers, they are.
>
> What i meant is that they (the system provider) are free to do that,
> you (the user) can not do that though.
>
> > > > Now you know where the __aligned define in my patch comes from. :-)
> > >
> > > You should conditionally include include sys/cdefs.h on NetBSD and not
> > > defining your own version of __aligned (which is forbidden by the
> > > standard).
> >
> > sys/cdefs.h is included implicitely by any libc standard header on
> > NetBSD. No need for an #ifdef __NetBSD__ ... #endif for this.
>
> Then i actually fail to see how your original patch is supposed to work
> at all, gcc should raise hell about redefinition of __aligned.
>
> > > Few more points:
> > >
> > >  a. the file in question already uses aligned attribute _inside_ the
> > >     structures without any macros
> >
> > So the pragmas have no effect then ? If so, then just remove them.
>
> Not really, the individual members are marked with attribute aligned
> (just take a look), not the whole structures.
>
> > >  b. unlike all other xen* files the header guard is __XEN_BLKIF_H__
> > >     and not QEMU_XEN_BLKIF_H (that should be fixed too)
> >
> > Easy to fix. Will you do or should I send a diff ?
>
> That would be very nice of you.

Is attached patch fine with you ?

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: qemu_xen_blkif.diff --]
[-- Type: text/x-diff, Size: 1032 bytes --]

diff --git a/hw/xen_blkif.h b/hw/xen_blkif.h
index 738b8fe..f47f9a4 100644
--- a/hw/xen_blkif.h
+++ b/hw/xen_blkif.h
@@ -1,5 +1,5 @@
-#ifndef __XEN_BLKIF_H__
-#define __XEN_BLKIF_H__
+#ifndef __QEMU_BLKIF_H__
+#define __QEMU_BLKIF_H__
 
 #include <xen/io/ring.h>
 #include <xen/io/blkif.h>
@@ -17,7 +17,6 @@ struct blkif_common_response {
 };
 
 /* i386 protocol version */
-#pragma pack(push, 4)
 struct blkif_x86_32_request {
 	uint8_t        operation;    /* BLKIF_OP_???                         */
 	uint8_t        nr_segments;  /* number of segments                   */
@@ -33,7 +32,6 @@ struct blkif_x86_32_response {
 };
 typedef struct blkif_x86_32_request blkif_x86_32_request_t;
 typedef struct blkif_x86_32_response blkif_x86_32_response_t;
-#pragma pack(pop)
 
 /* x86_64 protocol version */
 struct blkif_x86_64_request {
@@ -100,4 +98,4 @@ static void inline blkif_get_x86_64_req(blkif_request_t *dst, blkif_x86_64_reque
 		dst->seg[i] = src->seg[i];
 }
 
-#endif /* __XEN_BLKIF_H__ */
+#endif /* __QEMU_BLKIF_H__ */