-
Notifications
You must be signed in to change notification settings - Fork 278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use compiler macros for endianness #2688
Conversation
#ifdef __LITTLE_ENDIAN__ | ||
return true; | ||
#else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this macro (__LITTLE_ENDIAN__
) sometimes defined when __BYTE_ORDER__
isn't? If so, maybe this logic should be moved to isBigEndianPlatform
. I.e. add this to the beginning of isBigEndianPlatform
:
#if defined(__BIG_ENDIAN__)
return true;
#elif defined(__LITTLE_ENDIAN__)
return false;
#elif ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__LITTLE_ENDIAN__
is our own macro (potentially) defined in config.h
.
Everything else comes from the compiler. AFAICT, there is no definition of __BIG_ENDIAN__
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this macro (
__LITTLE_ENDIAN__
) sometimes defined when__BYTE_ORDER__
isn't?
Potentially on WIN32 and MSVC (which AFAIK does not define __BYTE_ORDER__
):
Line 43 in 50648dd
#if defined(_WIN32) || defined(__CYGWIN__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevinbackhouse I'm now leveraging these as well.
Codecov Report
@@ Coverage Diff @@
## main #2688 +/- ##
=======================================
Coverage 63.90% 63.90%
=======================================
Files 103 103
Lines 22313 22313
Branches 10799 10799
=======================================
Hits 14260 14260
Misses 5828 5828
Partials 2225 2225
|
082a87d
to
936cc01
Compare
936cc01
to
7930039
Compare
For Windows, it's always little endian AFAIK. |
Yes, and we set |
Anyway, a lot of work for this function. Would be nice to just use 84e45a3 Requires GCC9 though. |
I've been fine with this all along. 😉 |
@neheb I'm going to merge this for now as I believe it is more portable, so please rebase your change for eventual GCC 9 (and Clang 9/10?) bump... |
Supersedes #2668 as not fully portable.
Using the same form as in
include/config.h