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

[William Pitcock <nenolod@xxxxxxxxxxxxxxxx>]

Hi,

On Mon, Dec 19, 2011 at 3:48 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> 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?

uClibc 0.9.32 and later have byteswap.h.  Not sure about earlier versions.
byteswap.h is a GNU-ism though, and not in any of the BSDs as a result.

We should just ship these definitions directly in Xen and not depend on it.

William

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