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

unix::WindowExt no longer returns pointers for things that aren't actually pointers #364

Merged
merged 1 commit into from
Dec 17, 2017

Conversation

francesca64
Copy link
Member

Fixes #256

get_xlib_window and get_xlib_screen_id previously returned Option<*mut c_void> by casting integer IDs into pointers, which while producing no functionality issues, is semantically incorrect and rather surprising. Worse still, the docs for get_xlib_window stated that it was in fact a valid pointer.

This is a breaking change, and will require some trivial changes to glutin.

I chose to have get_xlib_window return Option<libc::c_ulong> rather than Option<ffi::Window>, since I assume ffi::Window isn't supposed to be exposed.

@tomaka
Copy link
Contributor

tomaka commented Dec 15, 2017

Thanks, the PR looks good.
A slight nit is that I'd prefer to use the types from std::os::raw instead of libc.

…ually pointers

Fixes rust-windowing#256

`get_xlib_window` and `get_xlib_screen_id` previously returned `Option<*mut c_void>` by
casting integer IDs into pointers, which while producing no functionality issues, is
semantically incorrect and rather surprising. Worse still, the docs for `get_xlib_window`
stated that it was in fact a valid pointer.

Additionally, now all `unix::WindowExt` methods return `std::os::raw` types rather than
`libc` types; note that the two versions of `c_void` are not interchangeable in the eyes of
the compiler, so those wanting the `libc` type will need to explicitly cast.

This is a breaking change, and will require some trivial changes to glutin.
@francesca64 francesca64 force-pushed the unix-windowext-pointers branch from 9e8af57 to 32d76ef Compare December 15, 2017 20:38
@francesca64
Copy link
Member Author

I've changed the other methods of unix::WindowExt to follow that convention as well.

@tomaka tomaka merged commit 23e4293 into rust-windowing:master Dec 17, 2017
francesca64 added a commit to francesca64/winit that referenced this pull request Dec 24, 2017
tmfink pushed a commit to tmfink/winit that referenced this pull request Jan 5, 2022
madsmtm pushed a commit to madsmtm/winit that referenced this pull request Jun 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants