Re: htonll, ntohll

["Torsten Bögershausen" <tboegi@web.de>]

On 2013-11-05 01.00, Ramsay Jones wrote:
> On 31/10/13 13:24, Torsten Bögershausen wrote:
>> On 2013-10-30 22.07, Ramsay Jones wrote:
> [ ... ]
>>> Yep, this was the first thing I did as well! ;-) (*late* last night)
>>>
>>> I haven't had time today to look into fixing up the msvc build
>>> (or a complete re-write), so I look forward to seeing your solution.
>>> (do you have msvc available? - or do you want me to look at fixing
>>> it? maybe in a day or two?)
>>>
>> Ramsay,
>> I don't have msvc, so feel free to go ahead, as much as you can.
>>
>> I'll send a patch for the test code I have made, and put bswap.h on hold for a week
>> (to be able to continue with t5601/connect.c)
> 
> Unfortunately, I haven't had much time to look into this.
> 
> I do have a patch (given below) that works on Linux, cygwin,
> MinGW and msvc. However, the msvc build is still broken (as a
> result of _other_ commits in this 'jk/pack-bitmap' branch; as
> well as the use of a VLA in another commit).
> 
> So, I still have work to do! :(
> 
> Anyway, I thought I would send what I have, so you can take a look.
> Note, that I don't have an big-endian machine to test this on, so
> YMMV. Indeed, the *only* testing I have done is to run the test added
> by this branch (t5310-pack-bitmaps.sh), which works on Linux, cygwin
> and MinGW.
> 
> [Note: I have never particularly liked htons, htonl et.al., so adding
> these htonll/ntohll functions doesn't thrill me! :-D For example see
> this post[1], which echo's my sentiments exactly.]
> 
> HTH
> 
> ATB,
> Ramsay Jones
> 
> [1] http://commandcenter.blogspot.co.uk/2012/04/byte-order-fallacy.html
> 
> -- >8 --
> Subject: [PATCH] compat/bswap.h: Fix build on cygwin, MinGW and msvc
> 
> ---
>  compat/bswap.h | 97 ++++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 68 insertions(+), 29 deletions(-)
> 
> diff --git a/compat/bswap.h b/compat/bswap.h
> index ea1a9ed..c18a78e 100644
> --- a/compat/bswap.h
> +++ b/compat/bswap.h
> @@ -17,7 +17,20 @@ static inline uint32_t default_swab32(uint32_t val)
>  		((val & 0x000000ff) << 24));
>  }
>  
> +static inline uint64_t default_bswap64(uint64_t val)
> +{
> +	return (((val & (uint64_t)0x00000000000000ffULL) << 56) |
> +		((val & (uint64_t)0x000000000000ff00ULL) << 40) |
> +		((val & (uint64_t)0x0000000000ff0000ULL) << 24) |
> +		((val & (uint64_t)0x00000000ff000000ULL) <<  8) |
> +		((val & (uint64_t)0x000000ff00000000ULL) >>  8) |
> +		((val & (uint64_t)0x0000ff0000000000ULL) >> 24) |
> +		((val & (uint64_t)0x00ff000000000000ULL) >> 40) |
> +		((val & (uint64_t)0xff00000000000000ULL) >> 56));
> +}
> +
>  #undef bswap32
> +#undef bswap64
>  
>  #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
>  
> @@ -32,54 +45,80 @@ static inline uint32_t git_bswap32(uint32_t x)
>  	return result;
>  }
>  
> +#define bswap64 git_bswap64
> +#if defined(__x86_64__)
> +static inline uint64_t git_bswap64(uint64_t x)
> +{
> +	uint64_t result;
> +	if (__builtin_constant_p(x))
> +		result = default_bswap64(x);
> +	else
> +		__asm__("bswap %q0" : "=r" (result) : "0" (x));
> +	return result;
> +}
> +#else
> +static inline uint64_t git_bswap64(uint64_t x)
> +{
> +	union { uint64_t i64; uint32_t i32[2]; } tmp, result;
> +	if (__builtin_constant_p(x))
> +		result.i64 = default_bswap64(x);
> +	else {
> +		tmp.i64 = x;
> +		result.i32[0] = git_bswap32(tmp.i32[1]);
> +		result.i32[1] = git_bswap32(tmp.i32[0]);
> +	}
> +	return result.i64;
> +}
> +#endif
> +
>  #elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))
>  
>  #include <stdlib.h>
>  
>  #define bswap32(x) _byteswap_ulong(x)
> +#define bswap64(x) _byteswap_uint64(x)
>  
>  #endif
>  
> -#ifdef bswap32
> +#if defined(bswap32)
>  
>  #undef ntohl
>  #undef htonl
>  #define ntohl(x) bswap32(x)
>  #define htonl(x) bswap32(x)
>  
> -#ifndef __BYTE_ORDER
> -#	if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && defined(BIG_ENDIAN)
> -#		define __BYTE_ORDER BYTE_ORDER
> -#		define __LITTLE_ENDIAN LITTLE_ENDIAN
> -#		define __BIG_ENDIAN BIG_ENDIAN
> -#	else
> -#		error "Cannot determine endianness"
> -#	endif
> +#endif
> +
> +#if defined(bswap64)
> +
> +#undef ntohll
> +#undef htonll
> +#define ntohll(x) bswap64(x)
> +#define htonll(x) bswap64(x)
> +
> +#else
> +
> +#undef ntohll
> +#undef htonll
> +
> +#if !defined(__BYTE_ORDER)
> +# if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && defined(BIG_ENDIAN)
> +#  define __BYTE_ORDER BYTE_ORDER
> +#  define __LITTLE_ENDIAN LITTLE_ENDIAN
> +#  define __BIG_ENDIAN BIG_ENDIAN
> +# endif
> +#endif
> +
> +#if !defined(__BYTE_ORDER)
> +# error "Cannot determine endianness"
>  #endif
>  
>  #if __BYTE_ORDER == __BIG_ENDIAN
>  # define ntohll(n) (n)
>  # define htonll(n) (n)
> -#elif __BYTE_ORDER == __LITTLE_ENDIAN
> -#	if defined(__GNUC__) && defined(__GLIBC__)
> -#		include <byteswap.h>
> -#	else /* GNUC & GLIBC */
> -static inline uint64_t bswap_64(uint64_t val)
> -{
> -	return ((val & (uint64_t)0x00000000000000ffULL) << 56)
> -		| ((val & (uint64_t)0x000000000000ff00ULL) << 40)
> -		| ((val & (uint64_t)0x0000000000ff0000ULL) << 24)
> -		| ((val & (uint64_t)0x00000000ff000000ULL) <<  8)
> -		| ((val & (uint64_t)0x000000ff00000000ULL) >>  8)
> -		| ((val & (uint64_t)0x0000ff0000000000ULL) >> 24)
> -		| ((val & (uint64_t)0x00ff000000000000ULL) >> 40)
> -		| ((val & (uint64_t)0xff00000000000000ULL) >> 56);
> -}
> -#	endif /* GNUC & GLIBC */
> -#	define ntohll(n) bswap_64(n)
> -#	define htonll(n) bswap_64(n)
> -#else /* __BYTE_ORDER */
> -#	error "Can't define htonll or ntohll!"
> +#else
> +# define ntohll(n) default_bswap64(n)
> +# define htonll(n) default_bswap64(n)
>  #endif
>  
>  #endif
> 
I have had time to test it, works on Linux/PPC (big endian)
and Mac OS.

What do we think about going ahead with this patch?
/Torsten