Skip to content

ndk,ndk-glue: Do not pass Looper ident as userdata pointer#112

Merged
MarijnS95 merged 2 commits into
rust-mobile:masterfrom
MarijnS95:poll-use-identifier
Jan 30, 2021
Merged

ndk,ndk-glue: Do not pass Looper ident as userdata pointer#112
MarijnS95 merged 2 commits into
rust-mobile:masterfrom
MarijnS95:poll-use-identifier

Conversation

@MarijnS95
Copy link
Copy Markdown
Member

The value of 0 or 1 is not a valid pointer to a userdata structure. Not that it has to be, but with the nasty consequence that winit Android code was accidentally reinterpreting this pointer as integer to extract the identifier from, instead of using the appropriate ident field directly. After that is addressed in winit we can set the pointer back to 0 cleanly (and prepare for a potential pointer to some user data that may be passed in the future).

Also add some constants to document these "special" identifiers.


Maintainers, this is a breaking change in ndk-glue that we want to bump our minor version for, to v0.3. Anyone upgrading now will always see null() in the data field, wrecking winit unless rust-windowing/winit#1826 is in, which will only land in 0.25.

After releasing this we should do a small patch to winit to use the new v0.3 crate together with the new constants for clarity.

CC @nwessing: the extra bit of documentation in here might help your adventure into using the glue crate directly!

Comment thread ndk-glue/src/lib.rs
Comment thread ndk/src/input_queue.rs
Comment thread ndk-glue/src/lib.rs Outdated
Comment thread ndk-glue/CHANGELOG.md Outdated
@MarijnS95 MarijnS95 force-pushed the poll-use-identifier branch from c9074f3 to 0a184e6 Compare January 28, 2021 18:38
@MarijnS95
Copy link
Copy Markdown
Member Author

@msiglreith I'm thinking about merging this and releasing a minor version bump this weekend, followed by an MR in winit to use the new version and constants. Any other breaking changes we might want to get in?

(Manifest changes - which I also intend to finally look at - are only build-related fortunately)

@MarijnS95 MarijnS95 force-pushed the poll-use-identifier branch from 0a184e6 to 2856b3a Compare January 28, 2021 18:46
@msiglreith
Copy link
Copy Markdown
Contributor

Sounds good to me. I think there is nothing else in the pipeline right now.

@MarijnS95 MarijnS95 force-pushed the poll-use-identifier branch from 2856b3a to 1711e85 Compare January 28, 2021 19:57
The value of `0` or `1` is not a valid pointer to a userdata structure.
Not that it has to be, but with the nasty consequence that winit Android
code was accidentally reinterpreting this pointer as integer to extract
the identifier from, instead of using the appropriate `ident` field
directly. After that is addressed in winit we can set the pointer back
to 0 cleanly (and prepare for a potential pointer to some user data that
may be passed in the future).

Also add some constants to document these "special" identifiers.
@MarijnS95 MarijnS95 force-pushed the poll-use-identifier branch from 1711e85 to f9bb13e Compare January 30, 2021 15:51
@MarijnS95
Copy link
Copy Markdown
Member Author

@msiglreith Shall we bump the version at the same time, or in one PR? Also, do we want to keep ndk-sys versioning in sync with ndk, even though no changes have been made?

@msiglreith
Copy link
Copy Markdown
Contributor

@MarijnS95 Doing the version bump in this PR is fine.

Also, do we want to keep ndk-sys versioning in sync with ndk, even though no changes have been made?

I would prefer to keep it at the current version in this case.

@MarijnS95
Copy link
Copy Markdown
Member Author

@msiglreith neat, so the 0.2.1 version match was just a coincidence, nothing intentional.

Let's merge this then :)

@MarijnS95 MarijnS95 merged commit 30d032a into rust-mobile:master Jan 30, 2021
@MarijnS95 MarijnS95 deleted the poll-use-identifier branch January 30, 2021 17:30
MarijnS95 added a commit to MarijnS95/winit that referenced this pull request Jan 30, 2021
Following the changes in [1] this bumps ndk and ndk-glue to 0.3 and uses
the new constants. The minor version has been bumped to prevent
applications from running an older winit (without rust-windowing#1826) with a newer
ndk/ndk-glue that does not pass this `ident` through the `data` pointer
anymore.

[1]: rust-mobile/ndk#112
MarijnS95 added a commit to rust-windowing/winit that referenced this pull request Jan 30, 2021
…1847)

Following the changes in [1] this bumps ndk and ndk-glue to 0.3 and uses
the new constants. The minor version has been bumped to prevent
applications from running an older winit (without #1826) with a newer
ndk/ndk-glue that does not pass this `ident` through the `data` pointer
anymore.

[1]: rust-mobile/ndk#112
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.

3 participants