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

[Andrew Bartlett]

On Mon, 2014-05-05 at 09:17 -0700, Christof Schmitt wrote:
> 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.

This patch set (attached) does exactly that, and just needs one more
smoke test and a team reviewer.

Can you double-check it with this patch set, by running these commands
for me on this system?

./configure

sort bin/default/include/config.h > /tmp/config.h.sorted
git am *.patch
./configure

sort bin/default/include/config.h > /tmp/config.h.sorted-new
diff -u /tmp/config.h.sorted /tmp/config.h.sorted-newer 

The only change should be this (or similar):
@@ -1105,5 +1105,4 @@
 /* #undef SYSCONF_SC_NPROC_ONLN */
 /* #undef USE_DMAPI */
 /* #undef USE_LINUX_32BIT_SYSCALLS */
-/* #undef WORDS_BIGENDIAN */
 /* #undef XATTR_ADDITIONAL_OPTIONS */

If so, can you push both this and whatever version of the byteorder.h
patch set Christof is happy with?

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba



-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-build-unify-and-fix-endian-tests.patch
Type: text/x-patch
Size: 7469 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140506/5fb0c96c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-build-make-wafsamba-CHECK_SIZEOF-cross-compile-frien.patch
Type: text/x-patch
Size: 2610 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140506/5fb0c96c/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-build-tweak-SIZEOF-utmp-ut_line.patch
Type: text/x-patch
Size: 1303 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140506/5fb0c96c/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-build-find-blkcnt_t-size-via-array.patch
Type: text/x-patch
Size: 2136 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140506/5fb0c96c/attachment-0003.bin>