[PATCH] byteorder: do not assume PowerPC is big-endian

[Christof Schmitt]

On Mon, May 05, 2014 at 04:58:58PM +0200, David Disseldorp wrote:
> byteorder.h currently uses reverse-indexing ASM instructions for little
> endian multi-byte storage/retrieval on PowerPC. With Power8 this is an
> incorrect assumption, as it can be big or little endian.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=10590
> 
> Signed-off-by: David Disseldorp <ddiss at samba.org>
> ---
>  lib/util/byteorder.h | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/util/byteorder.h b/lib/util/byteorder.h
> index 58cd68a..82c050b 100644
> --- a/lib/util/byteorder.h
> +++ b/lib/util/byteorder.h
> @@ -19,6 +19,7 @@
>  
>  #ifndef _BYTEORDER_H
>  #define _BYTEORDER_H
> +#include <ccan/endian/endian.h>
>  
>  /*
>     This file implements macros for machine independent short and 
> @@ -89,10 +90,12 @@ it also defines lots of intermediate macros, just ignore those :-)
>  
>  
>  /*
> -  on powerpc we can use the magic instructions to load/store
> -  in little endian
> -*/
> -#if (defined(__powerpc__) && defined(__GNUC__))
> + * On powerpc we can use the magic instructions to load/store in little endian.
> + * The instructions are reverse-indexing, so assume a big endian Power
> + * processor. Power8 can be big or little endian, so we need to explicitly
> + * check.
> + */
> +#if (defined(__powerpc__) && defined(__GNUC__) && HAVE_BIG_ENDIAN)
>  static __inline__ uint16_t ld_le16(const uint16_t *addr)
>  {
>  	uint16_t val;
> -- 
> 1.8.4.5

Hi David,

i saw the bugzilla review request. Without having access to a power
system, i can only review the code. It looks correct, but i saw that
there is already a endianess check in buildtools/wafsamba/wscript:

    conf.CHECK_CODE('long one = 1; return ((char *)(&one))[0]',
                    execute=True,
                    define='WORDS_BIGENDIAN')

You could use the WORDS_BIGENDIAN define instead of importing ccan. Of
course, it the longterm it would be good to only have one endianess
check, maybe the ccan one.

Christof