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.

Sign up for GitHub

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

Jump to bottom

Use I/O safety in sys::mman #1916

Merged
merged 1 commit into from
Dec 6, 2022
Merged

Use I/O safety in sys::mman #1916

merged 1 commit into from
Dec 6, 2022

Conversation

asomers
Copy link
Member

@asomers asomers commented Dec 5, 2022

No description provided.

@asomers asomers mentioned this pull request Dec 5, 2022
Open
29 tasks
@asomers asomers requested a review from rtzoeller December 6, 2022 19:39
rtzoeller
rtzoeller approved these changes Dec 6, 2022
View reviewed changes
Copy link
Collaborator

@rtzoeller rtzoeller left a comment

Choose a reason for hiding this comment

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

Approved, but with a question: how do we want to handle the changelog for these changes, which do modify API signatures?

@asomers
Copy link
Member Author

asomers commented Dec 6, 2022

Approved, but with a question: how do we want to handle the changelog for these changes, which do modify API signatures?

Since all of these changes are related, I think we should roll them up into one big CHANGELOG entry at the top of the file. The awesomest option would be if we could convince Cargo to print suggestions for how to fix users' code, at least for the common cases. But I don't know how.

@rtzoeller
Copy link
Collaborator

The awesomest option would be if we could convince Cargo to print suggestions for how to fix users' code, at least for the common cases. But I don't know how.

I'll do some digging to see if there are attributes we can use.

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 6, 2022

Build succeeded:

@bors bors bot merged commit 67f7d46 into nix-rust:master Dec 6, 2022
@asomers asomers deleted the mman-iosafety branch December 6, 2022 22:21
@SteveLauC SteveLauC mentioned this pull request Dec 9, 2022
Merged
bors bot added a commit that referenced this pull request Dec 9, 2022
@bors @SteveLauC
Merge #1932
e11646e 
1932: refactor: take `AsFd` by value r=asomers a=SteveLauC

#### What this PR does

1. Changes the `fd` type to take `AsFd` by value for the I/O safety PRs that are merged.
    * #1916 
    * #1919 
    * #1921 
    * #1922 

Co-authored-by: Steve Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rtzoeller rtzoeller rtzoeller approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@asomers @rtzoeller