Skip to content
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

Fallback to compiler defines when __BYTE_ORDER is not available #513

Merged
merged 3 commits into from
Aug 2, 2022

Conversation

kulikjak
Copy link
Contributor

@kulikjak kulikjak commented Aug 1, 2022

On some systems (I tested it on Oracle Solaris and FreeBSD), __BYTE_ORDER, __LITTLE_ENDIAN and __BIG_ENDIAN from arpa/inet.h are not available, which can cause failures on big endian systems (because little endian is the default). This adds a fallback to common compiler defines in those cases.

Addresses issues raised in #495.

@methane
Copy link
Member

methane commented Aug 1, 2022

After how __BYTE_ORDER and __BYTE_ORDER__ are used in world, I prefer using just __BYTE_ORDER__.
Please make it simple.

@kulikjak
Copy link
Contributor Author

kulikjak commented Aug 1, 2022

Done, thanks!

@methane
Copy link
Member

methane commented Aug 1, 2022

Remove this too:

#else
#include <arpa/inet.h>  /* __BYTE_ORDER */

@kulikjak
Copy link
Contributor Author

kulikjak commented Aug 1, 2022

I missed that - updated and retested.

@methane methane merged commit 9d45926 into msgpack:main Aug 2, 2022
@klemensn
Copy link

klemensn commented Dec 2, 2022

This fix is required on OpenBSD/sparc64 to unbreak borgbackup 1.2.2 at runtime, see borgbackup/borg#6149 (comment) for details.

@klemensn
Copy link

klemensn commented Dec 2, 2022

msgpack-python 1.0.4 regression tests pass 100% with and without this fix on OpenBSD/sparc64.
Here is an example test run of 1.0.4 without this fix: https://gist.github.com/936496c1bea568a0f741ae9b3ec54d39

Passing tests but failing runtime consumers at the same time suggests a lack of endianess coverage.

@klemensn
Copy link

klemensn commented Dec 3, 2022

Remove this too:

#else
#include <arpa/inet.h>  /* __BYTE_ORDER */

Can you elaborate why?

This include is required by ntohs(3) later in the same file:
https://github.com/msgpack/msgpack-python/pull/513/files#diff-5c123c916851553e85829468dc2fadee6b1ed6edda136c8d544d5a0eaf594d99R90

Accidentially, it seems, this builds fine on OpenBSD, but breaks at least in Ubuntu 20.04, see
borgbackup/borg#7181 (comment) and borgbackup/borg@e79986b.

@methane
Copy link
Member

methane commented Dec 3, 2022

#514

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.

3 participants