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

switch from winapi-rs to windows-sys #51

Closed
wants to merge 7 commits into from

Conversation

Philosobyte
Copy link
Contributor

@Philosobyte Philosobyte commented Mar 30, 2023

windows-sys is officially supported by Microsoft and contains many more APIs than winapi-rs does. Switching to windows-sys makes future work easier.
I started from @sn99's PR #48 and discovered the reason for the test failure. HKEY is defined in windows-sys as an isize. It is possible for HKEY_CLASSES_ROOT to overflow this isize and break comparisons in the RegKey::close_(&mut self) method, thus resulting in RegKeys not being dropped when they go out of scope. The easiest fix I could think of is to cast HKEYs to a usize during the comparison.

With all due respect to @sn99, I've removed most of the stylistic changes so the PR is more focused and easier for @gentoo90 to review.

I'm pretty new to both Windows development and Rust, so I would take any feedback vigorously.

@Philosobyte Philosobyte changed the title Use windows rs switch from winapi-rs to windows-sys-rs Mar 30, 2023
@Philosobyte Philosobyte changed the title switch from winapi-rs to windows-sys-rs switch from winapi-rs to windows-sys Mar 30, 2023
Cargo.toml Outdated
@@ -13,7 +13,14 @@ categories = ["api-bindings", "os::windows-apis"]

[dependencies]
cfg-if = "1.0"
winapi = { version = "0.3.9", features = ["impl-default", "impl-debug", "minwindef", "minwinbase", "timezoneapi", "winerror", "winnt", "winreg", "handleapi"] }
windows-sys = {version = "0.45.0", features = [
Copy link

Choose a reason for hiding this comment

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

windows-sys 0.48 was released some days ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I was relying on my IDE's autocomplete, which appears to be outdated. I've bumped the version number up. Tests still pass.

@Philosobyte
Copy link
Contributor Author

Philosobyte commented Apr 3, 2023

Hmm, it appears @gentoo90 is working on a branch to switch to windows-sys as we speak. I will close this PR and eagerly await for gentoo90's work.

@Philosobyte Philosobyte closed this Apr 3, 2023
@gentoo90
Copy link
Owner

gentoo90 commented Apr 3, 2023

Rebased onto the latest master and published as part of version 0.50.0

@raymundane
Copy link

Rebased onto the latest master and published as part of version 0.50.0

Thank you!

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.

5 participants