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

[Roger Pau Monnà <roger.pau@xxxxxxxxxxxxx>]

2011/12/19 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.

From what I saw _BYTESWAP_H is defined in uclibc and glibc (Solaris
and NetBSD have their own preprocessor logic, so this doesn't apply to
them). It's just a safeguard that avoids redefining the macros if
someone has included byteswap.h behind our backs.

> 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?

The solutions I see are:

 * Check if byteswap.h has been included (this patch).
 * undef bswap_* functions and define our own.
 * Include byteswap.h.

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