Skip to content

Cast pointers more carefully #2140

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

Merged
merged 2 commits into from
Oct 1, 2023
Merged

Cast pointers more carefully #2140

merged 2 commits into from
Oct 1, 2023

Conversation

asomers
Copy link
Member

@asomers asomers commented Sep 29, 2023

Prefer ptr::{cast, cast_const, cast_mut} over as. The latter makes it too easy to accidentally change both the type and mutability when changing only one was intended.

This exercise caught an unintended mutability cast in one function, the BSD version of sendfile. In this case there's no UB because it isn't possible for anything else to get a reference to the data that was incorrectly cast.

There was also a type cast that wasn't guaranteed to be correct (but probably was) due to memory layout guarantees in if_nametoindex.

@asomers
Copy link
Member Author

asomers commented Sep 29, 2023

cc @Jan561 I was inspired to do this by what I saw in your PR #2120 . I tried to avoid making any conflicts with that PR.

@asomers asomers force-pushed the ptr-cast branch 2 times, most recently from 8518de9 to f16f2e3 Compare September 30, 2023 00:18
@asomers asomers requested a review from SteveLauC September 30, 2023 02:07
Prefer ptr::{cast, cast_const, cast_mut} over `as`.  The latter makes it
too easy to accidentally change both the type and mutability when
changing only one was intended.

This exercise caught an unintended mutability cast in one function, the
BSD version of sendfile.  In this case there's no UB because it isn't
possible for anything else to get a reference to the data that was
incorrectly cast.

There was also a type cast that wasn't guaranteed to be correct (but
probably was) due to memory layout guarantees in if_nametoindex.
@asomers
Copy link
Member Author

asomers commented Sep 30, 2023

I made that one change to UnixCredentials, and also rebased on master.

@SteveLauC SteveLauC added this pull request to the merge queue Oct 1, 2023
Merged via the queue into nix-rust:master with commit e5b2df6 Oct 1, 2023
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.

2 participants