Document and shrink some unsafe blocks in tokio-util #4655
Conversation
| @@ -81,17 +81,24 @@ where | |||
| } | |||
|
|
|||
| // We're out of data. Try and fetch more data to decode | |||
| let addr = unsafe { | |||
There was a problem hiding this comment.
Shrinking this unsafe block seems good to me.
|
@Noah-Kennedy Note that this shouldn't be merged until we have more clarity on tokio-rs/bytes#548. Perhaps the comments related to that PR should just be removed. |
| // FIXME(https://github.com/tokio-rs/bytes/pull/548): Consider switching to the safe | ||
| // `chunk_mut().into::<&mut [MaybeUninit<u8>]>()` if this PR lands. | ||
| // | ||
| // Safety: `chunk_mut()` returns a `&mut UninitSlice`, and `UninitSlice` is a |
There was a problem hiding this comment.
this is kind of a minor side note, but i've noticed that Tokio contains a number of "safety" comments, but the capitalization of the word "safety" is not very consistent. i think we may want to make this more consistent so that it's easier to grep for all such "safety" comments in the repo.
out of interest, i did a quick search to see which capitalization was the most common:
:# eliza at noctis in tokio on master [$] via ⚙️ v1.60.0 in ❄️ nix-shell [impure]
:; for needle in "safety:" "SAFETY:" "Safety:"; do echo -n "$needle " ; rg "$needle" | wc -l ; done
safety: 51
SAFETY: 42
Safety: 126and it looks like "Safety:" is the clear winner, so this seems consistent with the rest of the code.
we should really consider changing the other "safety" comments to be consistent with this...
|
Thanks all! I've removed references to tokio-rs/bytes#548. |
This documents why it is safe to convert `bytes::UninitSlice` to `&mut [MaybeUninit<u8>]`, and shrinks one of the unsafe blocks to make these functions easier to audit.
Head branch was pushed to by a user without write access
Motivation
We are auditing a tokio update in https://fuchsia-review.googlesource.com/c/fuchsia/+/611683, and we thought it would save other auditors time if we documented and shrunk down some
unsafe {}blocks.Solution
This documents why it is safe to convert
bytes::UninitSliceto&mut [MaybeUninit<u8>], and shrinks one of the unsafe blocks to make these functions easier to audit.This also removes an out of date comment around converting a
&mut [MaybeUninit<u8>]to a&mut [u8], which predated the conversion over toReadBuf, and is no longer applicable to the code.