Skip to content

Conversation

@Low-power
Copy link

The endian.h header, as well as macros and functions declared in it, are not a part of Linux, but really provided by the GNU C Library (GLIBC), so endianness.h should test the __GLIBC__ macro instead, for this feature.
But since the __GLIBC__ macro is usually not predefined by compiler, but rather in GLIBC's own features.h only, there is a small chance that it didn't get defined by the time endianness.h get included; for example the user may be including endianness.h before any GLIBC headers. The workaround I considered is keeping the original __linux macro test, however this is not ideal because it made an assumption that all other C library implementations for Linux must support this nonstandard endian.h, in a GLIBC-compatible way.
This change enables using GLIBC endian.h to detect byte-order on non-Linux-based GNU systems, where compiler built-in __BYTE_ORDER__ isn't available for any reason.

@rofl0r
Copy link
Owner

rofl0r commented Sep 14, 2021

The endian.h header, as well as macros and functions declared in it, are not a part of Linux, but really provided by the GNU C Library (GLIBC), so endianness.h should test the GLIBC macro instead, for this feature.

it's also provided by musl and other linux libcs i'm aware of, so using linux makes sense.

This change enables using GLIBC endian.h to detect byte-order on non-Linux-based GNU systems, where compiler built-in BYTE_ORDER isn't available for any reason.

is this for GNU hurd ? or is this only a theoretical patch without a real usecase ?

@Low-power
Copy link
Author

is this for GNU hurd ? or is this only a theoretical patch without a real usecase ?

This is a GLIBC header, it will be installed on any GNU system; you can also see its copyright notice saying This file is part of the GNU C Library..
As a manual verification, I has checked GNU/Hurd, GNU/kFreeBSD, as well as GNU/kOpenSolaris systems, for its availability.

@rofl0r
Copy link
Owner

rofl0r commented Sep 15, 2021

This is a GLIBC header, it will be installed on any GNU system; you can also see its copyright notice saying This file is part of the GNU C Library..

let's agree on that it is a header that's provided by GLIBC, among others.

As a manual verification, I has checked GNU/Hurd, GNU/kFreeBSD, as well as GNU/kOpenSolaris systems, for its availability.

alright, but you still didn't answer me on whether you encountered an actual scenario in practice where none of the other detection mechanisms kicked in.

@Low-power
Copy link
Author

whether you encountered an actual scenario in practice where none of the other detection mechanisms kicked in.

Actually no; because all those environments I tested has a properly configured GCC that predefines __BYTE_ORDER__ and associated macros, endianness.h didn't really need to include a C library header to get byte-order detection macros.

@rofl0r
Copy link
Owner

rofl0r commented Sep 15, 2021

Actually no; because all those environments I tested has a properly configured GCC that predefines BYTE_ORDER and associated macros, endianness.h didn't really need to include a C library header to get byte-order detection macros.

in that case, i'd prefer not to make any changes - as you pointed out adding the __GLIBC__ thing would anyway only work in case another system header is included before including endianness.h, making this patch not only addressing a hypothetical usecase, but also fixing the hypothetical usecase unreliably.

@Low-power
Copy link
Author

Yes, this __GLIBC__ detection could be unreliable due to inclusion order and specific-platform differences; from my recent test, for example, the GNU/kFreeBSD system have this macro predefined by the compiler (not only in features.h):

$ gcc -E -dM -x c /dev/null | grep -F -e GLIBC -e BSD -e kernel -e linux
#define __GLIBC__ 1
#define __FreeBSD_kernel__ 1

But this is not the case on GNU/Linux:

$ gcc -E -dM -x c /dev/null | grep -F -e GLIBC -e BSD -e kernel -e linux
#define __linux 1
#define __linux__ 1
#define __gnu_linux__ 1
#define linux 1

As you said adding this detection may only be useful on a hypothetical usecase, but I think it could still benefit people who reading the source code, by making the detection mechanism more complete: they will knowning that endian.h will be available on GLIBC, in addition to almost all other C libraries written for Linux.

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.

2 participants