-
Notifications
You must be signed in to change notification settings - Fork 67
Properly detect Linux with glibc to detect what's not #1874
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
base: master
Are you sure you want to change the base?
Conversation
It looks like the O_CLOEXEC symbol isn't missing.
|
|
I merged the first commit. |
Possibly, but that doesn't prevent us to cleanup the code. We were already avoiding the *64 family outside of Linux, so that code changes nothing for other unices like macOS, etc. What this patch does is to avoid the *64 family on Linux when glibc isn't there. In the future we can add more tests to enable *64 functions on more systems. |
|
I tested macOS and |
28ea2f2 to
324d13b
Compare
|
It looks like we don't need |
|
The macOS manpage mentions things like |
slipher
left a comment
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.
Oops, found a pending comment from weeks ago.
| #elif defined(__linux__) && defined(__GLIBC__) | ||
| #define DAEMON_OPEN64 | ||
| #if defined(__GLIBC__) | ||
| #define _FILE_OFFSET_BITS 64 |
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.
I would expect this not to work correctly since when precompiled headers are used, the PCH is inserted before the file contents. Anyway that sort of thing would be better added from the build system.
Also please squash file offset size-related commits together.
Properly detect Linux with glibc to detect what's not, no need to list everything else.
It makes it easier to build for something else we currently don't support, either being another operating system, or Linux with another libc.
I yet again faced the problem of assuming Glibc on Linux when toying with Fil-C that uses musl by default. I do remember having faced similar problems in the past with other stuff. Android? Zig? I don't remember precisely, but I knew that code was fragile, maybe it's better this way.
I'm curious to see if the CI will be happy or not.
If that works, then adding new operating systems would even require less code, not needing to list every of them.