Re: [PATCH v2 1/1] include/endian.h: Remove bswap* duplicate definitions

[Razvan Deaconescu <razvan.deaconescu@xxxxxxxxx>]

Awesome, thanks for the review, Vlad and for marking them as awaiting
upstream. They're likely going to end up on GitHub soon.

Razvan

On Thu, Nov 19, 2020 at 11:37 PM Vlad-Andrei BĂDOIU
<vlad_andrei.badoiu@xxxxxx> wrote:
>
> Hi Razvan,
>
> This patch is great. I've tried it with the other clang patches and newlib is 
> working all right. Thank you!
>
> Reviewed-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxx>
>
> ----- Original Message -----
> From: "Razvan Deaconescu" <razvan.deaconescu@xxxxxxxxx>
> To: "minios-devel" <minios-devel@xxxxxxxxxxxxx>
> Cc: "felipe huici" <felipe.huici@xxxxxxxxx>, "Razvan Deaconescu" 
> <razvan.deaconescu@xxxxxxxxx>
> Sent: Friday, September 18, 2020 2:32:51 PM
> Subject: [PATCH v2 1/1] include/endian.h: Remove bswap* duplicate definitions
>
> Using Clang results in compilation errors regarding the `__bswapXY` and
> `__builtin_bswapXY` functions. There are simultaneously a definition of
> a macro `__bswapXY` and a definition of a function `__bswapXY`; GCC
> doesn't complain, but Clang does.
>
> This is happening in Unikraft because the Unikraft core libraries are
> using a minimized Unikraft newlib library[4], while the Unikraft app is
> using the upstream (complete) newlib library[5]. In the end there are
> two `endian.h` files: one in the Unikraft newlib[6], another one in the
> upstream newlib (<repo>/newlib/libc/include/machine/endian.h). The
> Unikraft newlib version of `endian.h` defines `__bswapXY` as inline
> functions, whereas the upstream newlib defines them as macros. This
> results in an error from Clang; GCC doesn't mind, though it's an issue
> having multiple definitions for the same thing (even if one is a macro
> or one is a function).
>
> This commit fixes the issue. It removes the `__bswapXY` parts from the
> Unikraft version of `endian.h` and includes the upstream version of
> `endian.h`.
>
> Signed-off-by: Razvan Deaconescu <razvan.deaconescu@xxxxxxxxx>
> ---
>  include/endian.h | 17 ++---------------
>  1 file changed, 2 insertions(+), 15 deletions(-)
>
> diff --git a/include/endian.h b/include/endian.h
> index 3c8a752..f09c312 100644
> --- a/include/endian.h
> +++ b/include/endian.h
> @@ -27,6 +27,8 @@
>  #ifndef _ENDIAN_H
>  #define _ENDIAN_H
>
> +#include <machine/endian.h>
> +
>  #define __LITTLE_ENDIAN 1234
>  #define __BIG_ENDIAN 4321
>  #define __PDP_ENDIAN 3412
> @@ -41,21 +43,6 @@
>
>  #include <stdint.h>
>
> -static inline uint16_t __bswap16(uint16_t __x)
> -{
> -       return __x<<8 | __x>>8;
> -}
> -
> -static inline uint32_t __bswap32(uint32_t __x)
> -{
> -       return __x>>24 | (__x>>8&0xff00) | (__x<<8&0xff0000) | __x<<24;
> -}
> -
> -static inline uint64_t __bswap64(uint64_t __x)
> -{
> -       return (__bswap32(__x)+0ULL)<<32 | __bswap32(__x>>32);
> -}
> -
>  #if __BYTE_ORDER == __LITTLE_ENDIAN
>  #define htobe16(x) __bswap16(x)
>  #define be16toh(x) __bswap16(x)
> --
> 2.17.1