Skip to content

Commit

Permalink
[byte_slice] Clean up docs and methods
Browse files Browse the repository at this point in the history 
Makes progress on #1692
  • Loading branch information
@joshlf
joshlf committed Sep 20, 2024
1 parent 782c12f commit 794fef7
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 73 deletions.
82 changes: 35 additions & 47 deletions src/byte_slice.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,20 @@ impl<B: ByteSlice + DerefMut> ByteSliceMut for B {}
/// # Safety
///
/// If `B: CopyableByteSlice`, then the dereference stability properties
/// required by `ByteSlice` (see that trait's safety documentation) do not only
/// hold regarding two calls to `b.deref()` or `b.deref_mut()`, but also hold
/// regarding `c.deref()` or `c.deref_mut()`, where `c` is produced by copying
/// `b`.
/// required by [`ByteSlice`] (see that trait's safety documentation) do not
/// only hold regarding two calls to `b.deref()` or `b.deref_mut()`, but also
/// hold regarding `c.deref()` or `c.deref_mut()`, where `c` is produced by
/// copying `b`.
pub unsafe trait CopyableByteSlice: ByteSlice + Copy + CloneableByteSlice {}

/// A [`ByteSlice`] which can be cloned without violating dereference stability.
///
/// # Safety
///
/// If `B: CloneableByteSlice`, then the dereference stability properties
/// required by `ByteSlice` (see that trait's safety documentation) do not only
/// hold regarding two calls to `b.deref()` or `b.deref_mut()`, but also hold
/// regarding `c.deref()` or `c.deref_mut()`, where `c` is produced by
/// required by [`ByteSlice`] (see that trait's safety documentation) do not
/// only hold regarding two calls to `b.deref()` or `b.deref_mut()`, but also
/// hold regarding `c.deref()` or `c.deref_mut()`, where `c` is produced by
/// `b.clone()`, `b.clone().clone()`, etc.
pub unsafe trait CloneableByteSlice: ByteSlice + Clone {}

Expand All @@ -101,61 +101,49 @@ pub unsafe trait CloneableByteSlice: ByteSlice + Clone {}
/// - `first`'s address is `addr` and its length is `split`
/// - `second`'s address is `addr + split` and its length is `len - split`
pub unsafe trait SplitByteSlice: ByteSlice {
/// Splits the slice at the midpoint.
/// Attempts to split `self` at the midpoint.
///
/// `x.split_at(mid)` returns `x[..mid]` and `x[mid..]`.
/// `s.split_at(mid)` returns `Ok((s[..mid], s[mid..]))` if `mid <=
/// s.deref().len()` and otherwise returns `Err(s)`.
///
/// # Panics
/// # Safety
///
/// `x.split_at(mid)` panics if `mid > x.deref().len()`.
#[must_use]
/// Unsafe code may rely on this function correctly implementing the above
/// functionality.
#[inline]
fn split_at(self, mid: usize) -> (Self, Self) {
if let Ok(splits) = try_split_at(self, mid) {
splits
#[must_use]
fn split_at(self, mid: usize) -> Result<(Self, Self), Self> {
if mid <= self.deref().len() {
// SAFETY: Above, we ensure that `mid <= self.deref().len()`. By
// invariant on `ByteSlice`, a supertrait of `SplitByteSlice`,
// `.deref()` is guranteed to be "stable"; i.e., it will always
// dereference to a byte slice of the same address and length. Thus,
// we can be sure that the above precondition remains satisfied
// through the call to `split_at_unchecked`.
unsafe { Ok(self.split_at_unchecked(mid)) }
} else {
panic!("mid > len")
Err(self)
}
}

/// Splits the slice at the midpoint, possibly omitting bounds checks.
///
/// `x.split_at_unchecked(mid)` returns `x[..mid]` and `x[mid..]`.
/// `s.split_at_unchecked(mid)` returns `s[..mid]` and `s[mid..]`.
///
/// # Safety
///
/// `mid` must not be greater than `x.deref().len()`.
/// `mid` must not be greater than `self.deref().len()`.
///
/// # Panics
///
/// Implementations of this method may choose to perform a bounds check and
/// panic if `mid > self.deref().len()`. They may also panic for any other
/// reason. Since it is optional, callers must not rely on this behavior for
/// soundness.
#[must_use]
unsafe fn split_at_unchecked(self, mid: usize) -> (Self, Self);
}

/// Attempts to split the slice at the midpoint.
///
/// `x.try_split_at(mid)` returns `Ok((x[..mid], x[mid..]))` if `mid <=
/// x.deref().len()` and otherwise returns `Err(x)`.
///
/// # Safety
///
/// Unsafe code may rely on this function correctly implementing the above
/// functionality.
#[inline]
pub(crate) fn try_split_at<S>(slice: S, mid: usize) -> Result<(S, S), S>
where
S: SplitByteSlice,
{
if mid <= slice.deref().len() {
// SAFETY: Above, we ensure that `mid <= self.deref().len()`. By
// invariant on `ByteSlice`, a supertrait of `SplitByteSlice`,
// `.deref()` is guranteed to be "stable"; i.e., it will always
// dereference to a byte slice of the same address and length. Thus, we
// can be sure that the above precondition remains satisfied through the
// call to `split_at_unchecked`.
unsafe { Ok(slice.split_at_unchecked(mid)) }
} else {
Err(slice)
}
}

/// A shorthand for [`SplitByteSlice`] and [`ByteSliceMut`].
pub trait SplitByteSliceMut: SplitByteSlice + ByteSliceMut {}
impl<B: SplitByteSlice + ByteSliceMut> SplitByteSliceMut for B {}
Expand All @@ -177,7 +165,7 @@ pub unsafe trait IntoByteSlice<'a>: ByteSlice {
/// # Safety
///
/// The returned reference has the same address and length as `self.deref()`
/// or `self.deref_mut()`.
/// and `self.deref_mut()`.
///
/// Note that, combined with the safety invariant on [`ByteSlice`], this
/// safety invariant implies that the returned reference is "stable" in the
Expand All @@ -202,7 +190,7 @@ pub unsafe trait IntoByteSliceMut<'a>: IntoByteSlice<'a> + ByteSliceMut {
/// # Safety
///
/// The returned reference has the same address and length as `self.deref()`
/// or `self.deref_mut()`.
/// and `self.deref_mut()`.
///
/// Note that, combined with the safety invariant on [`ByteSlice`], this
/// safety invariant implies that the returned reference is "stable" in the
Expand Down
46 changes: 20 additions & 26 deletions src/ref.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,12 @@ where
return Err(AlignmentError::new(bytes).into());
}
let (bytes, suffix) =
try_split_at(bytes, mem::size_of::<T>()).map_err(|b| SizeError::new(b).into())?;
bytes.split_at(mem::size_of::<T>()).map_err(|b| SizeError::new(b).into())?;
// SAFETY: We just validated alignment and that `bytes` is at least as
// large as `T`. `try_split_at(bytes, mem::size_of::<T>())?` ensures
// that the new `bytes` is exactly the size of `T`. By safety
// postcondition on `SplitByteSlice::try_split_at` we can rely on
// `try_split_at` to produce the correct `bytes` and `suffix`.
// large as `T`. `bytes.split_at(mem::size_of::<T>())?` ensures that the
// new `bytes` is exactly the size of `T`. By safety postcondition on
// `SplitByteSlice::split_at` we can rely on `split_at` to produce the
// correct `bytes` and `suffix`.
let r = unsafe { Ref::new_unchecked(bytes) };
Ok((r, suffix))
}
Expand All @@ -252,17 +252,16 @@ where
} else {
return Err(SizeError::new(bytes).into());
};
let (prefix, bytes) =
try_split_at(bytes, split_at).map_err(|b| SizeError::new(b).into())?;
let (prefix, bytes) = bytes.split_at(split_at).map_err(|b| SizeError::new(b).into())?;
if !util::aligned_to::<_, T>(bytes.deref()) {
return Err(AlignmentError::new(bytes).into());
}
// SAFETY: Since `split_at` is defined as `bytes_len - size_of::<T>()`,
// the `bytes` which results from `let (prefix, bytes) =
// try_split_at(bytes, split_at)?` has length `size_of::<T>()`. After
// bytes.split_at(split_at)?` has length `size_of::<T>()`. After
// constructing `bytes`, we validate that it has the proper alignment.
// By safety postcondition on `SplitByteSlice::try_split_at` we can rely
// on `try_split_at` to produce the correct `prefix` and `bytes`.
// By safety postcondition on `SplitByteSlice::split_at` we can rely on
// `split_at` to produce the correct `prefix` and `bytes`.
let r = unsafe { Ref::new_unchecked(bytes) };
Ok((prefix, r))
}
Expand Down Expand Up @@ -373,13 +372,11 @@ where
// underflow.
#[allow(unstable_name_collisions, clippy::incompatible_msrv)]
let split_at = unsafe { source.len().unchecked_sub(remainder.len()) };
let (bytes, suffix) =
try_split_at(source, split_at).map_err(|b| SizeError::new(b).into())?;
let (bytes, suffix) = source.split_at(split_at).map_err(|b| SizeError::new(b).into())?;
// SAFETY: `try_cast_into` validates size and alignment, and returns a
// `split_at` that indicates how many bytes of `source` correspond to a
// valid `T`. By safety postcondition on `SplitByteSlice::try_split_at`
// we can rely on `try_split_at` to produce the correct `source` and
// `suffix`.
// valid `T`. By safety postcondition on `SplitByteSlice::split_at` we
// can rely on `split_at` to produce the correct `source` and `suffix`.
let r = unsafe { Ref::new_unchecked(bytes) };
Ok((r, suffix))
}
Expand Down Expand Up @@ -431,13 +428,11 @@ where
};

let split_at = remainder.len();
let (prefix, bytes) =
try_split_at(source, split_at).map_err(|b| SizeError::new(b).into())?;
let (prefix, bytes) = source.split_at(split_at).map_err(|b| SizeError::new(b).into())?;
// SAFETY: `try_cast_into` validates size and alignment, and returns a
// `try_split_at` that indicates how many bytes of `source` correspond to
// a valid `T`. By safety postcondition on
// `SplitByteSlice::try_split_at` we can rely on `try_split_at` to
// produce the correct `prefix` and `bytes`.
// `split_at` that indicates how many bytes of `source` correspond to a
// valid `T`. By safety postcondition on `SplitByteSlice::split_at` we
// can rely on `split_at` to produce the correct `prefix` and `bytes`.
let r = unsafe { Ref::new_unchecked(bytes) };
Ok((prefix, r))
}
Expand Down Expand Up @@ -492,10 +487,7 @@ where
Some(len) => len,
None => return Err(SizeError::new(source).into()),
};
if source.len() < expected_len {
return Err(SizeError::new(source).into());
}
let (prefix, bytes) = source.split_at(expected_len);
let (prefix, bytes) = source.split_at(expected_len).map_err(SizeError::new)?;
Self::from_bytes(prefix).map(move |l| (l, bytes))
}

Expand All @@ -521,7 +513,9 @@ where
} else {
return Err(SizeError::new(source).into());
};
let (bytes, suffix) = source.split_at(split_at);
// SAFETY: The preceeding `source.len().checked_sub(expected_len)`
// guarantees that `split_at` is in-bounds.
let (bytes, suffix) = unsafe { source.split_at_unchecked(split_at) };
Self::from_bytes(suffix).map(move |l| (bytes, l))
}
}
Expand Down

0 comments on commit 794fef7

Please sign in to comment.