classification
Title: Add PY_BYTE_ORDER macro to get endianess of platform
Type: enhancement Stage: resolved
Components: Build Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, christian.heimes, haypo, jcea, ned.deily, pitrou, python-dev, rpetrov, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2012-10-08 14:33 by christian.heimes, last changed 2012-10-19 17:43 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
endianess_cleanup.patch serhiy.storchaka, 2012-10-08 17:22 review
endian.patch christian.heimes, 2012-10-10 09:42 review
endian2.patch christian.heimes, 2012-10-17 21:46 review
Messages (25)
msg172381 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-10-08 14:33
I propose the addition of three new macros in pyport.h

#define PY_LITTLE_ENDIAN 1234
#define PY_BIG_ENDIAN 4321
#define PY_BYTE_ORDER <either little or big endian>

that are either set by a configure test or implemented like brg_endian.h. pyconfig.h has WORDS_BIGENDIAN which just checks for __BIG_ENDIAN__. The sysmodule has its own C code that checks the first byte of a long to detect the endianess.

http://hg.python.org/cpython/file/dd5e98ddcd39/Modules/_sha3/keccak/brg_endian.h

I'd like to remove Keccak's brg_endian.h (#16113 SHA-3 module) for something more official and useful to other code. I had to re-add the file because the tests were failing on a big endian Solaris (Sparc) machine.
msg172395 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-08 16:48
I agree, it is worth bring order to work with endianess. We can get rod of BYTEORDER_IS_BIG_ENDIAN and BYTEORDER_IS_LITTLE_ENDIAN in Objects/unicodeobject.c, IS_LITTLE_ENDIAN in Objects/longobject.c, tests in other modules. I don't see the necessity in new macros. WORDS_BIGENDIAN is enough.

If something failing on a big endian Solaris, build script should just define WORDS_BIGENDIAN.
msg172397 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-08 17:22
Here is a patch which removes some unnecessary defines and tests.
msg172399 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-08 18:30
This doesn't depend on #16113 (quite the contrary).
msg172400 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-08 18:32
> #define PY_LITTLE_ENDIAN 1234
> #define PY_BIG_ENDIAN 4321
> #define PY_BYTE_ORDER <either little or big endian>

Ouch, sounds confusing. I would rather have PY_LITTLE_ENDIAN defined only on little-endian machines and PY_BIG_ENDIAN only on big-endian machines.
(and PY_BYTE_ORDER isn't necessary)
msg172401 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-08 18:50
> Ouch, sounds confusing. I would rather have PY_LITTLE_ENDIAN defined only
> on little-endian machines and PY_BIG_ENDIAN only on big-endian machines.
> (and PY_BYTE_ORDER isn't necessary)

Why use two complementary boolean variables for a single boolean value (Python 
does not support mixed endian in any case)? There is WORDS_BIGENDIAN already.
msg172403 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-08 19:02
> > Ouch, sounds confusing. I would rather have PY_LITTLE_ENDIAN defined only
> > on little-endian machines and PY_BIG_ENDIAN only on big-endian machines.
> > (and PY_BYTE_ORDER isn't necessary)
> 
> Why use two complementary boolean variables for a single boolean value (Python 
> does not support mixed endian in any case)?

Because it's simply more practical than having to remember which one
exists :-)

> There is WORDS_BIGENDIAN already.

A rather unfortunate IMHO.
msg172416 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2012-10-08 21:00
Just a reminder that we support configurations where there are both big-endian and little-endinan binaries in the *same* executable file: for example, Mac OS X 32-bit-only universal binaries (.so, .dylib, .exe) contain both i386 and ppc code in the same file.  So any -endian differences must remain isolated to C code or be dynamically tested at execution time in Python code. And ./configure time tests can be problematic.  (The proposed patch doesn't have that problem but it might be an issue later in tests.)
msg172427 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-10-09 00:45
My proposal mimics the API of endian.h. It defines a BYTE_ORDER macro that is either equal to LITTLE_ENDIAN, BIG_ENDIAN or PDB_ENDIAN (aka mixed endian). I need it that way for Keccak but I can roll my own definitions if you prefer just two macros.

If you prefer two macros instead of three then we should name them PY_IS_LITTLE_ENDIAN and PY_IS_BIG_ENDIAN.

Ned:
If I understand you correctly than we can't have a configure definition and need macro magic like brg_endian.h.
msg172431 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2012-10-09 01:38
Christian: That's right because there is only one configure execution in the OS X universal builds and only compiler call per module just like a normal single-architecture unix build.  Under the covers, the Apple compiler driver transparently makes multiple compiler calls, one for each architecture, and then use lipo to combine the binaries into one .o file etc.
msg172469 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-09 10:17
> If you prefer two macros instead of three then we should name them
> PY_IS_LITTLE_ENDIAN and PY_IS_BIG_ENDIAN.

Fine with me. Having three macros is pointless complication.

> If I understand you correctly than we can't have a configure definition 
> and need macro magic like brg_endian.h.

Replicating the current WORDS_BIGENDIAN logic should be enough.
msg172484 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-10-09 15:07
I think it's not enough as some platforms prefix/suffix the macros with dashes. The values must be compared to BYTE_ORDER iff the macro is defined. too. I've checked some machines of the Snakebite network and came up with this macro cascade:

/*
 * The endianess (byte order) can be defined in several ways. Some platforms
 * define either LITTLE_ENDIAN or BIG_ENDIAN while other platforms define
 * both and require comparison to BYTE_ORDER. Sometimes the macros are
 * prefixed or suffixed with one or two dashes, too. SPAM seems to be an
 * alias of __SPAM an all platforms that have SPAM.
 * The endian check can't be handled in a configure step as it would break
 * Apple's universal builds.
 */

#if defined(__BYTE_ORDER)
#  if defined(__BIG_ENDIAN) && __BYTE_ORDER == __BIG_ENDIAN
#    define PY_IS_BIG_ENDIAN 1
#  elif defined(__LITTLE_ENDIAN) && __BYTE_ORDER == __LITTLE_ENDIAN
#    define PY_IS_LITTLE_ENDIAN 1
#  else
#    error "__BYTE_ORDER not in (__BIG_ENDIAN, __LITTLE_ENDIAN)"
#  endif
#elif defined(__BIG_ENDIAN) && !defined(__LITTLE_ENDIAN)
#  define PY_IS_BIG_ENDIAN 1
#elif defined(__LITTLE_ENDIAN) && !defined(__BIG_ENDIAN)
#  define PY_IS_LITTLE_ENDIAN 1
/* and know the same block with just one dash as prefix */
#elif defined(_BYTE_ORDER)
#  if defined(_BIG_ENDIAN) && _BYTE_ORDER == _BIG_ENDIAN
#    define PY_IS_BIG_ENDIAN 1
#  elif defined(_LITTLE_ENDIAN) && _BYTE_ORDER == _LITTLE_ENDIAN
#    define PY_IS_LITTLE_ENDIAN 1
#  else
#    error "_BYTE_ORDER not in (_BIG_ENDIAN, _LITTLE_ENDIAN)"
#  endif
#elif defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN)
#  define PY_IS_BIG_ENDIAN 1
#elif defined(_LITTLE_ENDIAN) && !defined(_BIG_ENDIAN)
#  define PY_IS_LITTLE_ENDIAN 1
/* extra apple */
#elif defined(__APPLE__)
#  if defined(__BIG_ENDIAN__)
#    define PY_IS_BIG_ENDIAN 1
#  elif defined(__LITTLE_ENDIAN__)
#    define PY_IS_LITTLE_ENDIAN 1
#  else
#    error "__APPLE__ but neither __BIG_ENDIAN__ nor __LITTLE_ENDIAN__"
#  endif
/* Windows */
#elif defined(WIN32)
#  define PY_IS_LITTLE_ENDIAN 1
#else
#  error "Unable to detect endianess"
#endif
msg172491 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-09 16:32
> Fine with me. Having three macros is pointless complication.

Don't having two macros for one boolean value pointless complication too?
msg172499 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-09 18:37
> > Fine with me. Having three macros is pointless complication.
> 
> Don't having two macros for one boolean value pointless complication too?

No, since you need only one to get going. Either:

#ifdef PY_LITTLE_ENDIAN
...

or:

#ifdef PY_BIG_ENDIAN
...

So this is really a simplification compared to the awkward:

#ifdef PY_ENDIAN == BIG_ENDIAN
msg172503 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-09 18:51
Usually you need code for both alternatives.

#ifdef WORDS_BIGENDIAN
...
#else
...
#endif

What can be simpler? Of cause, using *two* macros is complicated. You need 
only one and second macros is unnecessary.
msg172522 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-10-09 20:42
AC_C_BIGENDIAN
msg172534 - (view) Author: Roumen Petrov (rpetrov) * Date: 2012-10-09 21:38
As Victor point configure script already check for endian (macro AC_C_BIGENDIAN) but you should ask python OS X guru to review . 
Macro is not adjusted to python needs as action for universal build is not defined yet. The default is to define AC_APPLE_UNIVERSAL_BUILD .

Otherwise WORDS_BIGENDIAN is defined for bigendian .
msg172557 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-10-10 09:42
The patch defines either PY_IS_BIG_ENDIAN or PY_IS_LITTLE_ENDIAN and sets PY_LITTLE_ENDIAN_FLAG to either 0 or 1 for some modules that require a boolean flag.
msg173205 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-17 21:09
I would prefer if you yanked PY_LITTLE_ENDIAN_FLAG. We don't need two different ways to do it.
msg173210 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-10-17 21:46
I've replaced the macros with PY_BIG_ENDIAN or PY_LITTLE_ENDIAN. Both are always defined and exactly one is set to 1, the other to 0.
msg173211 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-17 21:48
Looks fine to me. I suppose you'll have to check the big endian buildbots.
msg173213 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-17 21:52
New changeset d2127cdec10e by Christian Heimes in branch 'default':
Issue #16166: Add PY_LITTLE_ENDIAN and PY_BIG_ENDIAN macros and unified
http://hg.python.org/cpython/rev/d2127cdec10e
msg173235 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-18 07:35
> I've replaced the macros with PY_BIG_ENDIAN or PY_LITTLE_ENDIAN. Both are
> always defined and exactly one is set to 1, the other to 0.

It looks like a joke.

The programmer puts on the table by his bed two glasses. One with water, for 
the case of he will drink at night. Another empty, for the case of he will not 
drink.
msg173348 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2012-10-19 17:27
Instead of using macro values, I would rather define ONLY a single macro, PY_LITTLE_ENDIAN or PY_BIG_ENDIAN.

Far less error prone!.

It is far fool-proof "#ifdef PY_LITTLE_ENDIAN" than "#if PY_LITTLE_ENDIAN".
msg173349 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-10-19 17:43
> Instead of using macro values, I would rather define ONLY a single macro,
> PY_LITTLE_ENDIAN or PY_BIG_ENDIAN.

Time machine strikes back.

#ifdef WORDS_BIGENDIAN
History
Date User Action Args
2012-10-19 17:43:13serhiy.storchakasetmessages: + msg173349
2012-10-19 17:27:40jceasetmessages: + msg173348
2012-10-18 07:35:59serhiy.storchakasetmessages: + msg173235
2012-10-17 22:25:55christian.heimessetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2012-10-17 21:52:29python-devsetnosy: + python-dev
messages: + msg173213
2012-10-17 21:48:15pitrousetmessages: + msg173211
stage: needs patch -> commit review
2012-10-17 21:46:20christian.heimessetfiles: + endian2.patch

messages: + msg173210
2012-10-17 21:09:36pitrousetmessages: + msg173205
2012-10-10 09:42:51christian.heimessetfiles: + endian.patch

messages: + msg172557
2012-10-10 00:24:47jceasetnosy: + jcea
2012-10-09 21:38:44rpetrovsetnosy: + rpetrov
messages: + msg172534
2012-10-09 20:42:49hayposetnosy: + haypo
messages: + msg172522
2012-10-09 18:51:07serhiy.storchakasetmessages: + msg172503
2012-10-09 18:37:45pitrousetmessages: + msg172499
2012-10-09 16:32:25serhiy.storchakasetmessages: + msg172491
2012-10-09 15:30:14Arfreversetnosy: + Arfrever
2012-10-09 15:07:42christian.heimessetmessages: + msg172484
2012-10-09 10:17:10pitrousetmessages: + msg172469
2012-10-09 01:38:59ned.deilysetmessages: + msg172431
2012-10-09 00:45:49christian.heimessetmessages: + msg172427
2012-10-08 21:00:19ned.deilysetnosy: + ned.deily
messages: + msg172416
2012-10-08 19:02:21pitrousetmessages: + msg172403
2012-10-08 18:50:16serhiy.storchakasetmessages: + msg172401
2012-10-08 18:32:11pitrousetmessages: + msg172400
2012-10-08 18:30:46pitrousetdependencies: - Add SHA-3 and SHAKE (Keccak) support
messages: + msg172399
2012-10-08 17:22:52serhiy.storchakasetfiles: + endianess_cleanup.patch
keywords: + patch
messages: + msg172397
2012-10-08 16:48:31serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg172395
2012-10-08 14:33:55christian.heimessetnosy: + pitrou
dependencies: + Add SHA-3 and SHAKE (Keccak) support
2012-10-08 14:33:27christian.heimescreate