On Tue, Jun 27 2017, Junio C. Hamano jotted: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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 Yes, this would be better, but I don't know what macro test to use to test for Solaris. Oracle documents defined(sun), but that doesn't work on the Solaris versions we tested, and looking/searching illumos headers I didn't find anything obvious. The __sun define is ours, and would work for us, but upstream couldn't take it. >> 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.
