-
Notifications
You must be signed in to change notification settings - Fork 720
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
Future-proof the kevent ABI #1572
Conversation
FreeBSD 12 adds some new private fields to struct kevent, and it changes the type of "data" from intptr_t to i64. For now libc only binds the 11-compat ABI, but that will change some day. Adjust mio's structure definitions for compatibility with either ABI. This should work on all 32 and 64 bit platforms.
target_os = "ios", | ||
target_os = "macos" | ||
))] | ||
type Data = libc::intptr_t; |
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.
DragonFlyBSD and MacOS (and likely iOS) still use intptr_t
in the manual. I think to be safe we should stick to that as well. You can just move FreeBSD down to NetBSD and OpenBSD.
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.
Except that FreeBSD 11 also still uses intptr_t
. And currently there's no way for a crate to select which libc ABI it wants to use. So by removing that type entirely we'll just rely on type inference.
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.
But you're using as i64
in multiple places below, so it doesn't use type interference.
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.
Yeah. I changed the argument type of kevent_register
to use i64 everywhere, instead of being platform-dependent. So now there's only one place that needs type inference: line 284 (and line 61, before my second commit). Those are the only places that actually use the libc type.
@@ -60,6 +49,7 @@ macro_rules! kevent { | |||
fflags: 0, | |||
data: 0, | |||
udata: $data as UData, | |||
..unsafe { mem::zeroed() } |
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.
Could you add a note that this is needed for FreeBSD?
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.
Sure. Except, looking at it again, I realize that I should've just deleted the fflags and data field initializers.
Thanks @asomers. |
FreeBSD 12 adds some new private fields to struct kevent, and it changes
the type of "data" from intptr_t to i64. For now libc only binds the
11-compat ABI, but that will change some day. Adjust mio's structure
definitions for compatibility with either ABI.
This should work on all 32 and 64 bit platforms.