[PATCH 3/3] compat/bswap.h: Detect endianness from XL C compiler macros

2014-10-26 Thread David Michael
There is no /usr/include/endian.h equivalent on z/OS, but the
compiler will define macros to indicate endianness on host and
target hardware.  This adds a test for these macros as a last
resort for determining byte order.

Signed-off-by: David Michael fedora@gmail.com
---
 compat/bswap.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/compat/bswap.h b/compat/bswap.h
index f6fd9a6..7fed637 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -122,6 +122,10 @@ static inline uint64_t git_bswap64(uint64_t x)
 #  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
 # elif defined(_LITTLE_ENDIAN)  !defined(_BIG_ENDIAN)
 #  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
+# elif defined(__THW_BIG_ENDIAN__)  !defined(__THW_LITTLE_ENDIAN__)
+#  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
+# elif defined(__THW_LITTLE_ENDIAN__)  !defined(__THW_BIG_ENDIAN__)
+#  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
 # else
 #  error Cannot determine endianness
 # endif
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] compat/bswap.h: Detect endianness from XL C compiler macros

2014-10-26 Thread Jeff King
On Sun, Oct 26, 2014 at 01:34:26PM -0400, David Michael wrote:

 diff --git a/compat/bswap.h b/compat/bswap.h
 index f6fd9a6..7fed637 100644
 --- a/compat/bswap.h
 +++ b/compat/bswap.h
 @@ -122,6 +122,10 @@ static inline uint64_t git_bswap64(uint64_t x)
  #  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
  # elif defined(_LITTLE_ENDIAN)  !defined(_BIG_ENDIAN)
  #  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
 +# elif defined(__THW_BIG_ENDIAN__)  !defined(__THW_LITTLE_ENDIAN__)
 +#  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
 +# elif defined(__THW_LITTLE_ENDIAN__)  !defined(__THW_BIG_ENDIAN__)
 +#  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN

Out of curiosity, is there ever a case where _both_ are defined? That
is, would:

diff --git a/compat/bswap.h b/compat/bswap.h
index f6fd9a6..b78a0bd 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -122,6 +122,10 @@ static inline uint64_t git_bswap64(uint64_t x)
 #  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
 # elif defined(_LITTLE_ENDIAN)  !defined(_BIG_ENDIAN)
 #  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
+# elif defined(__THW_BIG_ENDIAN__)
+#  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
+# elif defined(__THW_LITTLE_ENDIAN__)
+#  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
 # else
 #  error Cannot determine endianness
 # endif

be enough, or is that used to mark some other special case? Even if not,
there is not much downside to the more thorough conditions you put
above, but as a reviewer I wondered if there is something else going on.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] compat/bswap.h: Detect endianness from XL C compiler macros

2014-10-26 Thread David Michael
On Sun, Oct 26, 2014 at 2:38 PM, Jeff King p...@peff.net wrote:
 On Sun, Oct 26, 2014 at 01:34:26PM -0400, David Michael wrote:

 diff --git a/compat/bswap.h b/compat/bswap.h
 index f6fd9a6..7fed637 100644
 --- a/compat/bswap.h
 +++ b/compat/bswap.h
 @@ -122,6 +122,10 @@ static inline uint64_t git_bswap64(uint64_t x)
  #  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
  # elif defined(_LITTLE_ENDIAN)  !defined(_BIG_ENDIAN)
  #  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
 +# elif defined(__THW_BIG_ENDIAN__)  !defined(__THW_LITTLE_ENDIAN__)
 +#  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
 +# elif defined(__THW_LITTLE_ENDIAN__)  !defined(__THW_BIG_ENDIAN__)
 +#  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN

 Out of curiosity, is there ever a case where _both_ are defined? That
 is, would:

 diff --git a/compat/bswap.h b/compat/bswap.h
 index f6fd9a6..b78a0bd 100644
 --- a/compat/bswap.h
 +++ b/compat/bswap.h
 @@ -122,6 +122,10 @@ static inline uint64_t git_bswap64(uint64_t x)
  #  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
  # elif defined(_LITTLE_ENDIAN)  !defined(_BIG_ENDIAN)
  #  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
 +# elif defined(__THW_BIG_ENDIAN__)
 +#  define GIT_BYTE_ORDER GIT_BIG_ENDIAN
 +# elif defined(__THW_LITTLE_ENDIAN__)
 +#  define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN
  # else
  #  error Cannot determine endianness
  # endif

 be enough, or is that used to mark some other special case? Even if not,
 there is not much downside to the more thorough conditions you put
 above, but as a reviewer I wondered if there is something else going on.

I'm not 100% sure if __THW_LITTLE_ENDIAN__ will ever be defined, so
I'd be okay with dropping those references completely.  There is a
recent version of the compiler for little endian Linux distributions,
but I haven't found the documentation for it.  (The product
documentation still seems to only refer to the big endian Linux
version.)  The compiler's macro may be redundant in this case anyway,
since Linux systems should have bits/endian.h supplying that
information.

I only used both macros for completeness; the __THW_BIG_ENDIAN__ macro
(defined to 1 on z/OS and AIX) is what I actually needed here.  z/OS
doesn't seem to have any other compile-time byte order indicator,
short of testing for the OS itself.

Would you prefer the two-line patch to only test for the big endian
macro, or maybe just test for __MVS__ to look at the OS?

Thanks.

David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] compat/bswap.h: Detect endianness from XL C compiler macros

2014-10-26 Thread Jeff King
On Sun, Oct 26, 2014 at 04:04:01PM -0400, David Michael wrote:

 I'm not 100% sure if __THW_LITTLE_ENDIAN__ will ever be defined, so
 I'd be okay with dropping those references completely.  There is a
 recent version of the compiler for little endian Linux distributions,
 but I haven't found the documentation for it.  (The product
 documentation still seems to only refer to the big endian Linux
 version.)  The compiler's macro may be redundant in this case anyway,
 since Linux systems should have bits/endian.h supplying that
 information.
 
 I only used both macros for completeness; the __THW_BIG_ENDIAN__ macro
 (defined to 1 on z/OS and AIX) is what I actually needed here.  z/OS
 doesn't seem to have any other compile-time byte order indicator,
 short of testing for the OS itself.

Thanks for the explanation.

 Would you prefer the two-line patch to only test for the big endian
 macro,

I think that's OK, as long as the #else case barfs as it does now (i.e.,
doesn't silently choose little-endian).

 or maybe just test for __MVS__ to look at the OS?

If the OS's you're testing on all provide a big-endian marker like
__THW_BIG_ENDIAN__, that seems preferable to me to use, as it's more
explicit. We can deal with similar little-endian systems if and when
somebody sees one in the wild (and your explanation that it would
probably hit the Linux bits/endian.h check first makes sense to me).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html