Re: [PATCH 1/3] sha1dc: update from my PR #36
- From
- Liam R. Howlett <liam.howlett@oracle.com>
- Date
- Jun 27, 2017, 19:38 UTC
- Message-ID
- <20170627193835.goix6woeil3hfkhe@oracle.com>
- In-Reply-To
- <xmqq7ezx1c31.fsf@gitster.mtv.corp.google.com>
* Junio C Hamano <gitster@pobox.com> [170627 15:10]:
Show 27 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > > > Because in the current code is, abbreviated: > > > > #if (defined(_BYTE_ORDER) || defined(__BYTE_ORDER) || defined(__BYTE_ORDER__)) > > #if /* byte order is bendian */ > > #define SHA1DC_BIGENDIAN > > #endif > > #else > > #if /*some processor tests/* || defined(__sparc)) > > #define SHA1DC_BIGENDIAN > > #endif > > > > And since Solaris defines _BYTE_ORDER we never get to checking __sparc, > > and in fact the "/* byte order is bendian */" test errors out. > > > > This is fixed by my patch, where we first check gcc settings, then > > glibc, then processors, and finally _BYTE_ORDER (but as discussed that > > last bit could be adjusted to sun && _BYTE_ORDER, if we can find what > > "sun" is. > > Well, if Solaris defines _BYTE_ORDER, doesn't that mean they define > two constants _BIG_ENDIAN and _LITTLE_ENDIAN to compare it with? If > that is the case, I suspect that the change to make "comparison > between __BYTE_ORDER and __BIG_ENDIAN for GCC only" is going in a > wrong direction, as it wants to take the same approach as GCC, but > just uses a slightly different symbol.
That's not the case. _BIG_ENDIAN is defined but not to a value. I believe _BYTE_ORDER is defined as 4321, but _BIG_ENDIAN is just defined. This is why I proposed detecting the empty state of _BIG_ENDIAN. If _BIG_ENDIAN is defined but not to a value, then we already know it's big endian. I believe your approach below can be altered slightly to account for this issue.
Show 31 quoted lines
> > I wonder if the approach like the following might be cleaner to > extend as we find other oddball platforms. > > #undef __SHA1DC_BYTE_ORDER > #if defined(_BYTE_ORDER) > #define __SHA1DC_BYTE_ORDER _BYTE_ORDER > #elif defined(__BYTE_ORDER) > #define __SHA1DC_BYTE_ORDER __BYTE_ORDER > #elif defined(__BYTE_ORDER__)) > #define __SHA1DC_BYTE_ORDER __BYTE_ORDER__ > #endif > > #ifdef __SHA1DC_BYTE_ORDER > #undef __SHA1DC_BIG_ENDIAN > /* do the same for variations of BIG_ENDIAN constant */ > #if defined(_BIG_ENDIAN) > ... > #endif > > #if __SHA1DC_BYTE_ORDER == __SHA1DC_BIG_ENDIAN > #define SHA1DC_BIGENDIAN > #endif > #else > /* > * as the platform does not use "compare BYTE-ORDER with > * BIG_ENDIAN macro" strategy, defined-ness of BIG_ENDIAN > * may be usable as a sign that it is a big-endian box. > */ > #endif >