Re: [PATCH] Add extra logic required to detect endianness on Solaris

[Junio C Hamano <gitster@pobox.com>]

Charles Bailey <cbailey32@bloomberg.net> writes:

>  #if !defined(__BYTE_ORDER)
> +/* Known to be needed on Solaris but designed to potentially more portable */
> +
> +#if !defined(__BIG_ENDIAN)
> +#define __BIG_ENDIAN 4321
> +#endif
> +
> +#if !defined(__LITTLE_ENDIAN)
> +#define __LITTLE_ENDIAN 1234
> +#endif
> +
> +#if defined(_BIG_ENDIAN)
> +#define __BYTE_ORDER __BIG_ENDIAN
> +#endif
> +#if defined(_LITTLE_ENDIAN)
> +#define __BYTE_ORDER __LITTLE_ENDIAN
> +#endif

The existing support is only for platforms where all three macros
(BYTE_ORDER, LITTLE_ENDIAN and BIG_ENDIAN) are defined, and the
convention used on such platforms where BYTE_ORDER is set to either
one of the *_ENDIAN macros to tell the code which byte order we
have.  This mimics the convention where __BYTE_ORDER and other two
macros are already defined with two leading underscores, and in such
a case we do not have to do anything.  We make the final decision to
use or bypass bswap64() in our ntohll() implementation based on the
variables with double leading underscores.

This patch seems to address two unrelated issues in that.

 (1) The existing support does not help a platform where the
     convention is to define either _BIG_ENDIAN (with one leading
     underscore) or _LITTLE_ENDIAN and not both, which is Solaris
     but there may be others.

 (2) There may be __LITTLE_ENDIAN and __BIG_ENDIAN macros already
     defined on the platform.  Or these may not have been defined at
     all.  You avoid unconditionally redefing these.

I find the latter iffy.

What is the reason for avoiding redefinition?  Is it because you
know the original values they have are precious?  And if so in what
way they are precious?  If the reason of avoiding redefinition is
because you do not even know what their values are (so that you are
trying to be safe by preserving), what other things can you say
about their values you are preserving?

Specifically, do you know that these two are defined differently, so
that defining __BYTE_ORDER to one of them and comparing it to
__BIG_ENDIAN is a good way to tell if the platform is big endian?

I would understand it if (2) were "we undefine if these are defined
and then define them as 4321 and 1234 respectively, in order to
avoid a compiler warning against redefinition of a macro", but that
is not what I am seeing, so I am not sure what you meant to achieve
by that "if !defined()" constructs.

Thanks.