-
Notifications
You must be signed in to change notification settings - Fork 298
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
Switch BufMut::bytes_mut
from MaybeUninit
to &mut UninitSlice
, a type ownec by bytes
for this purpose
#433
Changes from 11 commits
9bad867
b8fe6f5
252a31a
d0ab1de
8fea13c
00b6d89
8853151
e7ae35c
a8fdfd1
200ed46
38e2fe1
1a87a33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,174 @@ | ||
use core::fmt; | ||
use core::mem::MaybeUninit; | ||
use core::ops::{ | ||
Index, IndexMut, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive, | ||
}; | ||
|
||
/// Uninitialized byte slice. | ||
/// | ||
/// Returned by `BufMut::bytes_mut()`, the referenced byte slice may be | ||
/// uninitialized. The wrapper provides safe access without introducing | ||
/// undefined behavior. | ||
/// | ||
/// The safety invariants of this wrapper are: | ||
/// | ||
/// 1. Reading from an `UninitSlice` is undefined behavior. | ||
/// 2. Writing uninitialized bytes to an `UninitSlice` is undefined behavior. | ||
/// | ||
/// The difference between `&mut UninitSlice` and `&mut [MaybeUninit<u8>]` is | ||
/// that it is possible in safe code to write uninitialized bytes to an | ||
/// `&mut [MaybeUninit<u8>]`, which this type prohibits. | ||
#[repr(transparent)] | ||
pub struct UninitSlice([MaybeUninit<u8>]); | ||
|
||
impl UninitSlice { | ||
/// Create a `&mut UninitSlice` from a pointer and a length. | ||
/// | ||
/// # Safety | ||
/// | ||
/// The caller must ensure that `ptr` references a valid memory region owned | ||
/// by the caller representing a byte slice for the duration of `'a`. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use bytes::buf::UninitSlice; | ||
/// | ||
/// let bytes = b"hello world".to_vec(); | ||
/// let ptr = bytes.as_ptr() as *mut _; | ||
/// let len = bytes.len(); | ||
/// | ||
/// let slice = unsafe { UninitSlice::from_raw_parts_mut(ptr, len) }; | ||
/// ``` | ||
pub unsafe fn from_raw_parts_mut<'a>(ptr: *mut u8, len: usize) -> &'a mut UninitSlice { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we have a constructor that takes an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe (also) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both sounds good to me. I think we can do that in a follow up PR. |
||
let maybe_init: &mut [MaybeUninit<u8>] = | ||
core::slice::from_raw_parts_mut(ptr as *mut _, len); | ||
&mut *(maybe_init as *mut [MaybeUninit<u8>] as *mut UninitSlice) | ||
} | ||
|
||
/// Write a single byte at the specified offset. | ||
/// | ||
/// # Panics | ||
/// | ||
/// The function panics if `index` is out of bounds. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use bytes::buf::UninitSlice; | ||
/// | ||
/// let mut data = [b'f', b'o', b'o']; | ||
/// let slice = unsafe { UninitSlice::from_raw_parts_mut(data.as_mut_ptr(), 3) }; | ||
/// | ||
/// slice.write_byte(0, b'b'); | ||
/// | ||
/// assert_eq!(b"boo", &data[..]); | ||
/// ``` | ||
pub fn write_byte(&mut self, index: usize, byte: u8) { | ||
assert!(index < self.len()); | ||
|
||
unsafe { self.as_mut_ptr().offset(index as isize).write(byte) }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could in theory overflow, even if unlikely. I'd suggest writing |
||
} | ||
|
||
/// Writes the contents of `src` into `self`. | ||
/// | ||
/// # Panics | ||
/// | ||
/// The function panics if `self` has a different length than `src`. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use bytes::buf::UninitSlice; | ||
/// | ||
/// let mut data = [b'f', b'o', b'o']; | ||
/// let slice = unsafe { UninitSlice::from_raw_parts_mut(data.as_mut_ptr(), 3) }; | ||
/// | ||
/// slice.write_slice(b"bar"); | ||
/// | ||
/// assert_eq!(b"bar", &data[..]); | ||
/// ``` | ||
pub fn write_slice(&mut self, src: &[u8]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this allow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question, I suspect allowing writes of shorter slices would be more practical for consumers but if we decide to have same-length-slice then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will rename this to |
||
use core::ptr; | ||
|
||
assert_eq!(self.len(), src.len()); | ||
|
||
unsafe { | ||
ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len()); | ||
} | ||
} | ||
|
||
/// Return a raw pointer to the slice's buffer. | ||
/// | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest documenting safety:
(also aliasing rules, but that should be obvious) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically, writing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah right! Way subtle |
||
/// # Safety | ||
/// | ||
/// The caller **must not** read from the referenced memory and **must not** | ||
/// write uninitialized bytes to the slice either. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Putting emphasis on "uninitialized" might be nice? |
||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use bytes::BufMut; | ||
/// | ||
/// let mut data = [0, 1, 2]; | ||
/// let mut slice = &mut data[..]; | ||
/// let ptr = BufMut::bytes_mut(&mut slice).as_mut_ptr(); | ||
/// ``` | ||
pub fn as_mut_ptr(&mut self) -> *mut u8 { | ||
self.0.as_mut_ptr() as *mut _ | ||
} | ||
|
||
/// Returns the number of bytes in the slice. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// use bytes::BufMut; | ||
/// | ||
/// let mut data = [0, 1, 2]; | ||
/// let mut slice = &mut data[..]; | ||
/// let len = BufMut::bytes_mut(&mut slice).len(); | ||
/// | ||
/// assert_eq!(len, 3); | ||
/// ``` | ||
pub fn len(&self) -> usize { | ||
self.0.len() | ||
} | ||
} | ||
|
||
impl fmt::Debug for UninitSlice { | ||
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
fmt.debug_struct("UninitSlice[...]").finish() | ||
} | ||
} | ||
|
||
macro_rules! impl_index { | ||
($($t:ty),*) => { | ||
$( | ||
impl Index<$t> for UninitSlice { | ||
type Output = UninitSlice; | ||
|
||
fn index(&self, index: $t) -> &UninitSlice { | ||
let maybe_uninit: &[MaybeUninit<u8>] = &self.0[index]; | ||
unsafe { &*(maybe_uninit as *const [MaybeUninit<u8>] as *const UninitSlice) } | ||
} | ||
} | ||
|
||
impl IndexMut<$t> for UninitSlice { | ||
fn index_mut(&mut self, index: $t) -> &mut UninitSlice { | ||
let maybe_uninit: &mut [MaybeUninit<u8>] = &mut self.0[index]; | ||
unsafe { &mut *(maybe_uninit as *mut [MaybeUninit<u8>] as *mut UninitSlice) } | ||
} | ||
} | ||
)* | ||
}; | ||
} | ||
|
||
impl_index!( | ||
Range<usize>, | ||
RangeFrom<usize>, | ||
RangeFull, | ||
RangeInclusive<usize>, | ||
RangeTo<usize>, | ||
RangeToInclusive<usize> | ||
); |
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.
It would be good to say what the concrete invariant/protocol for this type is, and in particular how it differs from
MaybeUninit
.if I understand correctly, the key difference is that the contents may be uninitialized, but the API ensures that once they are initialized, they will never be de-initialized again. Is that correct?
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.
Yes, that's correct. Especially it allows constructing
UninitSlice
from already-initialized slice (&mut [u8]
).