-
Notifications
You must be signed in to change notification settings - Fork 281
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
impl From for &mut UninitSlice
to &mut [MaybeUninit<u8>]
#548
Conversation
Sorry, but this is not sound. You can reborrow a |
More generally, you need to be able to pass a |
Ah good point, I didn't consider reborrowing, yeah that would let us deinitialize Would it be worthwhile to instead add a separate |
I'm fine with adding an unsafe method for converting to As for reborrowing, it really does give overlapping More fundamentally, the reason its unsound to do this is that the code that created the Note that Tokio already has a stable variant of |
Ah, that's great to know. It'd be great to document this, since it'd be important to check for this invariant during reviews. Would you be okay with the name
Oh sure, I just figured that when we stabilize std::io::ReadBuf, we might want to directly integrate support into the |
d66a2b0
to
39aa8fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that's great to know. It'd be great to document this, since it'd be important to check for this invariant during reviews.
Would you be able to add that to this PR?
src/buf/uninit_slice.rs
Outdated
@@ -124,6 +124,29 @@ impl UninitSlice { | |||
self.0.as_mut_ptr() as *mut _ | |||
} | |||
|
|||
/// Return a `&mut [MaybeUninit<u8>] to this slice's buffer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Return a `&mut [MaybeUninit<u8>] to this slice's buffer. | |
/// Return a `&mut [MaybeUninit<u8>]` to this slice's buffer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks.
This adds an unsafe method to convert a `&mut UninitSlice` into a `&mut [MaybeUninit<u8>]`. This method is unsafe because some of the bytes in the slice may be initialized, and the caller should not overwrite them with uninitialized bytes. This came about when auditing [tokio-util's udp frame], where they want to pass the unitialized portion of a `BytesMut` to [ReadBuf::uninit]. They need to do this unsafe pointer casting in a few places, which complicates audits. This method lets us document the safety invariants the caller needs to maintain when doing this conversion. [tokio-util's udp frame]: https:/tokio-rs/tokio/blob/master/tokio-util/src/udp/frame.rs#L87 [ReadBuf::uninit]: https://docs.rs/tokio/latest/tokio/io/struct.ReadBuf.html#method.uninit
Thanks for the review! I pushed up a new version that hopefully fixes your concerns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
/// ``` | ||
#[inline] | ||
pub unsafe fn as_uninit_slice_mut<'a>(&'a mut self) -> &'a mut [MaybeUninit<u8>] { | ||
&mut *(self as *mut _ as *mut [MaybeUninit<u8>]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a subtle reason this is not just &mut self.0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I didn't realize that this was possible when approving the PR.
This adds an unsafe method to convert a `&mut UninitSlice` into a `&mut [MaybeUninit<u8>]`. This method is unsafe because some of the bytes in the slice may be initialized, and the caller should not overwrite them with uninitialized bytes. This came about when auditing [tokio-util's udp frame], where they want to pass the unitialized portion of a `BytesMut` to [ReadBuf::uninit]. They need to do this unsafe pointer casting in a few places, which complicates audits. This method lets us document the safety invariants the caller needs to maintain when doing this conversion. [tokio-util's udp frame]: https:/tokio-rs/tokio/blob/master/tokio-util/src/udp/frame.rs#L87 [ReadBuf::uninit]: https://docs.rs/tokio/latest/tokio/io/struct.ReadBuf.html#method.uninit
This adds an unsafe method to convert a
&mut UninitSlice
into a&mut [MaybeUninit<u8>]
. This method is unsafe because some of the bytes in the slice may be initialized, and the caller should not overwrite them with uninitialized bytes.This came about when auditing tokio-util's udp frame, where they want to pass the unitialized portion of a
BytesMut
to ReadBuf::uninit. They need to do this unsafe pointer casting in a few places, which complicates audits. This method lets us document the safety invariants the caller needs to maintain when doing this conversion.