Skip to content

Harden byte order detection#154

Open
kkaefer wants to merge 2 commits into
masterfrom
kk/endian-detection
Open

Harden byte order detection#154
kkaefer wants to merge 2 commits into
masterfrom
kk/endian-detection

Conversation

@kkaefer

@kkaefer kkaefer commented Jul 1, 2026

Copy link
Copy Markdown
Member

Use __BYTE_ORDER__ for GCC/Clang, and include Linux/BSD-specific headers for endian detection, and a constexpr static_assert instead of silently defaulting to little-endian.

Fixes #135.

@kkaefer kkaefer requested a review from a team as a code owner July 1, 2026 09:09
@kkaefer kkaefer requested a review from joto July 1, 2026 09:09
@kkaefer

kkaefer commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

Clang and GCC define __BYTE_ORDER__ by default: https://gcc.gnu.org/onlinedocs/gcc-4.9.0/cpp/Common-Predefined-Macros.html

kkaefer added 2 commits July 1, 2026 12:00
Use __BYTE_ORDER__ for GCC/Clang, and include Linux/BSD-specific headers for endian detection.

Fixes #135.
Introduce new CI jobs for building and testing on big-endian platforms (s390x and sparc64) as well as FreeBSD.
@kkaefer kkaefer force-pushed the kk/endian-detection branch from 0e2208d to 47f522d Compare July 1, 2026 11:59
@joto

joto commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

If the is_little_endian() function can perfectly detect the endianness at compile time, why don't we get rid of all the macro shenenigans alltogether? The only thing this is used for is to call (or not) the byteswap_inplace() function. If we add an if (!is_little_endian()) condition inside that function, we should be okay. We can use if consexpr if we switch to c++17 (which I don't want to do because libosmium is still c++14, but it's not out of the question to do that step). But even with a simple if the compiler should be able to remove all that code in the little-endian case because it detects that is superfluous.

@kkaefer

kkaefer commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

If the is_little_endian() function can perfectly detect the endianness at compile time, why don't we get rid of all the macro shenenigans alltogether?

We can't. It didn't work, and I removed it again after running actual big endian machines where this failed.

@joto

joto commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

We can't. It didn't work, ...

Okay, then I am fine with it once the CI problems are fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config.hpp's endian test silently falls back to little-endian

2 participants