Re: [Xen-devel] [PATCH 1 of 5] blktap: remove HAVE_BYTESWAP_H checking, since it's defined by qemu

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Sun, 2011-12-18 at 12:48 +0000, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> # Date 1324171782 -3600
> # Node ID eed78eb655c40b0ac9af1b14c1cc03204f696b0b
> # Parent  b783e76e63a99c9d87fca4974492f60af99a2e7a
> blktap: remove HAVE_BYTESWAP_H checking, since it's defined by qemu
> 
> blktap was using defines set by qemu, even when the qemu config file
> is not included. Remove this checkings, and instead check if the file
> has been included before defining the necessary macros.
> 
> The output error is:
> 
> In file included from block-qcow.c:37:0:
> bswap.h:23:0: error: "bswap_16" redefined [-Werror]
> /usr/include/byteswap.h:30:0: note: this is the location of the
> previous definition
> bswap.h:31:0: error: "bswap_32" redefined [-Werror]
> /usr/include/byteswap.h:33:0: note: this is the location of the
> previous definition
> bswap.h:41:0: error: "bswap_64" redefined [-Werror]
> /usr/include/byteswap.h:37:0: note: this is the location of the
> previous definition
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> 
> diff -r b783e76e63a9 -r eed78eb655c4 tools/blktap/drivers/bswap.h
> --- a/tools/blktap/drivers/bswap.h    Sun Dec 18 02:29:42 2011 +0100
> +++ b/tools/blktap/drivers/bswap.h    Sun Dec 18 02:29:42 2011 +0100
> @@ -15,9 +15,7 @@
>  #define bswap_64(x) swap64(x)
>  #else
>  
> -#ifdef HAVE_BYTESWAP_H
> -#include <byteswap.h>
> -#else
> +#ifndef _BYTESWAP_H

This is basically saying "if the user hasn't already include byteswap.h"
but it seems to rely on that header using the precise guard _BYTESWAP_H
which I presume can differ across platforms. I don't think this is a
viable approach.

Given that we have our own definitions of these things any why not just
remove the ifdef and single the other #include of byteswap.h and always
use the ones we defined?

The other alternative is to remove our own version and always include
byteswap.h.

The right answer depends on how standardised byteswap.h is. It seems to
be in glibc but is it in uclibc and the BSD libcs? I can't find a
definitive reference but it seems like it is specified by POSIX?

Ian.

>  
>  #define bswap_16(x) \
>  ({ \
> @@ -51,7 +49,7 @@
>               (uint64_t)(((uint64_t)(__x) & (uint64_t)0xff00000000000000ULL) 
> >> 56) )); \
>  })
>  
> -#endif /* !HAVE_BYTESWAP_H */
> +#endif /* !_BYTESWAP_H */
>  
>  static inline uint16_t bswap16(uint16_t x)
>  {
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel