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

getpagesize() was removed from OSX (and posix) #177

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

partouf
Copy link
Contributor

@partouf partouf commented Sep 28, 2024

PAGE_SIZE is a macro that can be used (which I'm guessing is defined to be sysconf(_SC_PAGESIZE) probably)

https://stackoverflow.com/questions/37897645/page-size-undeclared-c

PAGE_SIZE is a macro that can be used (which I'm guessing is defined to be `sysconf(_SC_PAGESIZE)` probably)
@jeremy-rifkin
Copy link
Owner

Thanks for the PR! The SO post suggests that PAGE_SIZE is only available through kernel headers, do you know if that's still correct? I do see it in /usr/include/x86_64-linux-gnu/sys/user.h but that file also has a note about it being solely for gdb. I'm also not sure about #ifdef getpagesize, at least on my system this doesn't work. Would it be best to just replace the getpagesize call with sysconf(_SC_PAGESIZE)? I'm not seeing any mention of _SC_PAGESIZE in documentation for macos. Worst-case, using sysctl as outlined here https://opensource.apple.com/source/Libc/Libc-262/gen/getpagesize.c.auto.html seems reasonable.

@partouf
Copy link
Contributor Author

partouf commented Sep 28, 2024

PAGE_SIZE didn't give warnings for me that it would be undefined, but OSX is literally all over the place per minor OS update, I wouldn't be surprised
maybe using sysconf(_SC_PAGESIZE) would be best
you could check for __APPLE__ and _SC_PAGESIZE to be defined

(btw _SC_PAGESIZE is a POSIX thing, not Mac specific https://stackoverflow.com/questions/37193749/getpagesize-vs-sysconf-sc-pagesize)

@partouf
Copy link
Contributor Author

partouf commented Sep 28, 2024

I will try that later tonight

@partouf
Copy link
Contributor Author

partouf commented Sep 28, 2024

Ok I think this makes a little bit more sense, but I also don't like it.
The issue I have with the macro PAGE_SIZE is that it's too easily defined by literally anyone and set to something else but the actual page_size, it would be better to check for the existence of getpagesize(), but not sure how

@partouf
Copy link
Contributor Author

partouf commented Sep 29, 2024

ok this makes more sense I think, use the new way if possible, otherwise fallback to the old ways

ofcourse the reason why getpagesize works on linux is because glibc tries to be backwards compatible with the middleages, and posix is embedded into that, Apple doesn't give a crap about backwards compatibility

Copy link
Owner

@jeremy-rifkin jeremy-rifkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much, lgtm! I think this is probably the best way to go about it.

@jeremy-rifkin jeremy-rifkin merged commit ce8214b into jeremy-rifkin:main Sep 30, 2024
78 of 79 checks passed
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