Skip to content

Commit

Permalink
Merge pull request #103 from Manishearth/read-soundness
Browse files Browse the repository at this point in the history
Remove read(), add read_u32() and read_u64() and use everywhere.
  • Loading branch information
Alexhuszagh authored Sep 8, 2024
2 parents ec7edaf + 2680d09 commit 788bf99
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 18 deletions.
2 changes: 1 addition & 1 deletion lexical-parse-float/src/slow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ macro_rules! round_up_nonzero {

// First try reading 8-digits at a time.
if iter.is_contiguous() {
while let Some(value) = iter.read::<u64>() {
while let Some(value) = iter.read_u64() {
// SAFETY: safe since we have at least 8 bytes in the buffer.
unsafe { iter.step_by_unchecked(8) };
if value != 0x3030_3030_3030_3030 {
Expand Down
4 changes: 2 additions & 2 deletions lexical-parse-integer/src/algorithm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ where
debug_assert!(Iter::IS_CONTIGUOUS);

// Read our digits, validate the input, and check from there.
let bytes = u32::from_le(iter.read::<u32>()?);
let bytes = u32::from_le(iter.read_u32()?);
if is_4digits::<FORMAT>(bytes) {
// SAFETY: safe since we have at least 4 bytes in the buffer.
unsafe { iter.step_by_unchecked(4) };
Expand Down Expand Up @@ -289,7 +289,7 @@ where
debug_assert!(Iter::IS_CONTIGUOUS);

// Read our digits, validate the input, and check from there.
let bytes = u64::from_le(iter.read::<u64>()?);
let bytes = u64::from_le(iter.read_u64()?);
if is_8digits::<FORMAT>(bytes) {
// SAFETY: safe since we have at least 8 bytes in the buffer.
unsafe { iter.step_by_unchecked(8) };
Expand Down
9 changes: 7 additions & 2 deletions lexical-util/src/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,14 @@ pub trait BytesIter<'a>: Iterator<Item = &'a u8> {
/// many bytes as the size of V.
unsafe fn read_unchecked<V>(&self) -> V;

/// Try to read a value of a different type from the iterator.
/// Try to read a the next four bytes as a u32.
/// This advances the internal state of the iterator.
fn read<V>(&self) -> Option<V>;
fn read_u32(&self) -> Option<u32>;


/// Try to read the next eight bytes as a u64
/// This advances the internal state of the iterator.
fn read_u64(&self) -> Option<u64>;

/// Advance the internal slice by `N` elements.
///
Expand Down
32 changes: 25 additions & 7 deletions lexical-util/src/noskip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl<'a, const __: u128> Bytes<'a, __> {
/// # Safety
///
/// Safe as long as the number of the buffer is contains as least as
/// many bytes as the size of V.
/// many bytes as the size of V, and V is valid for all bit patterns.
#[inline]
#[allow(clippy::assertions_on_constants)]
pub unsafe fn read_unchecked<V>(&self) -> V {
Expand All @@ -117,13 +117,26 @@ impl<'a, const __: u128> Bytes<'a, __> {
unsafe { ptr::read_unaligned::<V>(slc.as_ptr() as *const _) }
}

/// Try to read a value of a different type from the iterator.
/// Try to read a the next four bytes as a u32.
/// This advances the internal state of the iterator.
#[inline]
pub fn read<V>(&self) -> Option<V> {
if Self::IS_CONTIGUOUS && self.as_slice().len() >= mem::size_of::<V>() {
pub fn read_u32(&self) -> Option<u32> {
if Self::IS_CONTIGUOUS && self.as_slice().len() >= mem::size_of::<u32>() {
// SAFETY: safe since we've guaranteed the buffer is greater than
// the number of elements read.
// the number of elements read. u32 is valid for all bit patterns
unsafe { Some(self.read_unchecked()) }
} else {
None
}
}

/// Try to read the next eight bytes as a u64
/// This advances the internal state of the iterator.
#[inline]
pub fn read_u64(&self) -> Option<u64> {
if Self::IS_CONTIGUOUS && self.as_slice().len() >= mem::size_of::<u64>() {
// SAFETY: safe since we've guaranteed the buffer is greater than
// the number of elements read. u64 is valid for all bit patterns
unsafe { Some(self.read_unchecked()) }
} else {
None
Expand Down Expand Up @@ -288,8 +301,13 @@ impl<'a: 'b, 'b, const __: u128> BytesIter<'a> for BytesIterator<'a, 'b, __> {
}

#[inline]
fn read<V>(&self) -> Option<V> {
self.byte.read()
fn read_u32(&self) -> Option<u32> {
self.byte.read_u32()
}

#[inline]
fn read_u64(&self) -> Option<u64> {
self.byte.read_u64()
}

#[inline]
Expand Down
23 changes: 17 additions & 6 deletions lexical-util/src/skip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ impl<'a, const FORMAT: u128> Bytes<'a, FORMAT> {
/// # Safety
///
/// Safe as long as the number of the buffer is contains as least as
/// many bytes as the size of V.
/// many bytes as the size of V, and V is valid for all bit patterns.
#[inline]
pub unsafe fn read_unchecked<V>(&self) -> V {
debug_assert!(Self::IS_CONTIGUOUS);
Expand All @@ -439,19 +439,30 @@ impl<'a, const FORMAT: u128> Bytes<'a, FORMAT> {
unsafe { ptr::read_unaligned::<V>(slc.as_ptr() as *const _) }
}

/// Try to read a value of a different type from the iterator.
/// Try to read a the next four bytes as a u32.
/// This advances the internal state of the iterator.
#[inline]
pub fn read<V>(&self) -> Option<V> {
if Self::IS_CONTIGUOUS && self.as_slice().len() >= mem::size_of::<V>() {
unsafe fn read_u32(&self) -> Option<u32> {
if Self::IS_CONTIGUOUS && self.as_slice().len() >= mem::size_of::<u32>() {
// SAFETY: safe since we've guaranteed the buffer is greater than
// the number of elements read.
// the number of elements read. u32 is valid for all bit patterns
unsafe { Some(self.read_unchecked()) }
} else {
None
}
}
/// Try to read a the next four bytes as a u32.
/// This advances the internal state of the iterator.
#[inline]
unsafe fn read_u64(&self) -> Option<u64> {
if Self::IS_CONTIGUOUS && self.as_slice().len() >= mem::size_of::<u64>() {
// SAFETY: safe since we've guaranteed the buffer is greater than
// the number of elements read. u64 is valid for all bit patterns
unsafe { Some(self.read_unchecked()) }
} else {
None
}
}

/// Check if the next element is a given value.
#[inline]
pub fn first_is(&mut self, value: u8) -> bool {
Expand Down

0 comments on commit 788bf99

Please sign in to comment.