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

Include relevant parts of ntapi v.0.3.7 within mio #1712

Merged
merged 5 commits into from
Aug 23, 2023

Conversation

FrankenApps
Copy link

This is an alternative / follow-up to #1707.

I add the relevant parts of ntapi v.0.3.7 to mio v.0.7.x in order to get rid of rejected code.

More specifically the code in question were these two lines (here and here) which triggered the unaligned_references error that is a hard error since Rust version 1.68.

Include the relevant parts of ntapi v.0.3.7 in mio v.0.7.x in order to get rid of rejected code.
@Thomasdezeeuw
Copy link
Collaborator

@Darksonn @carllerche what do you guys think about including this much code from another crate? Is this allowed licensing wise? I suppose it's just struct and function definition found in Windows, but I'm not a lawyer.

@Darksonn
Copy link
Contributor

Licensing-wise it is fine. The code in ntapi is available under the MIT license, and the code in mio is available under the MIT license. So including code from ntapi in mio is okay.

Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

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

Got a CI rustfmt, but otherwise LGTM.

@FrankenApps
Copy link
Author

Alright, should be good to go now.
I used the cfg_net! macro inconsistently, which is why rustfmt was not happy with it.

@Thomasdezeeuw Thomasdezeeuw merged commit abd2c5c into tokio-rs:v0.7.x Aug 23, 2023
16 checks passed
@Thomasdezeeuw
Copy link
Collaborator

Thanks @FrankenApps

@FrankenApps FrankenApps deleted the vendor-ntapi branch November 18, 2023 20:39
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