Junio C Hamano <gitster@xxxxxxxxx> writes: > Charles Bailey <cbailey32@xxxxxxxxxxxxx> 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. Just a thought. I am wondering if you may want to go the other way around. That is, instead of using "we have byte-order, big and little and the way to determine endianness is to see byte-order matches which of the latter two", use "there may be either big or little but not both defined, and that is how you learn the byte-order". And make these two macros match what Solaris happens to use. I am not sure which variant I like better myself, though. compat/bswap.h | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/compat/bswap.h b/compat/bswap.h index 120c6c1..e87998e 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -101,19 +101,24 @@ static inline uint64_t git_bswap64(uint64_t x) #undef ntohll #undef htonll -#if !defined(__BYTE_ORDER) -# if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && defined(BIG_ENDIAN) -# define __BYTE_ORDER BYTE_ORDER -# define __LITTLE_ENDIAN LITTLE_ENDIAN -# define __BIG_ENDIAN BIG_ENDIAN -# endif +#if !defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN) + +#if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && defined(BIG_ENDIAN) +# if BYTE_ORDER == BIG_ENDIAN +# define _BIG_ENDIAN +# else +# define _LITTLE_ENDIAN +#endif + #endif -#if !defined(__BYTE_ORDER) +#if !defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN) # error "Cannot determine endianness" +#elif defined(_BIG_ENDIAN) && defined(_LITTLE_ENDIAN) +# error "Your endianness is screwed up" #endif -#if __BYTE_ORDER == __BIG_ENDIAN +#if defined (_BIG_ENDIAN) # define ntohll(n) (n) # define htonll(n) (n) #else -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html