Re: [PATCH 1/3] sha1dc: update from my PR #36

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 27, 2017, 15:53 UTC
Message-ID: <xmqqinjh2zqu.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqmv8t317c.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 14 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> +#else /* Not under GCC-alike or glibc or <processor whitelist>  or <processor blacklist> */
>> +
>> +#ifdef _BIG_ENDIAN
>> +/*
>> + * Solaris / illumos defines either _LITTLE_ENDIAN or _BIG_ENDIAN in
>> + * <sys/isa_defs.h>.
>> + */
>> +#define SHA1DC_BIGENDIAN
>
> This makes readers of this patch wonder why we assume platforms
> won't define _LITTLE_ENDIAN and _BIG_ENDIAN at the same time, just
> like we saw in the section with __BIG_ENDIAN above.

To be a bit more constructive, I'd feel it MUCH safer, if this "If _BIG_ENDIAN is defined, set SHA1DC_BIGENDIAN" is done _ONLY_ when we definitively KNOW that we are on Solaris, something like:

	#if defined(__sun) && defined(_BIG_ENDIAN)
	/*
	 * Solaris ...
	 */
	#define SHA1DC_BIGENDIAN
	#endif

> Thanks, but this is starting to feel like watching a whack-a-mole
> played while blindfolded.  At some point, somebody upstream should
> declare that enough is enough and introduce the "SHA1DC_FORCE_ENDIAN" 
> macro.