Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions src/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use core::borrow::Borrow;
use core::convert::TryInto;
use core::iter::FusedIterator;
use core::mem::MaybeUninit;
use core::str;
#[cfg(feature = "std")]
use std::io;
Expand Down Expand Up @@ -47,14 +48,39 @@ impl<'a> HexToBytesIter<HexDigitsIter<'a>> {
Self::from_pairs(HexDigitsIter::new_unchecked(s.as_bytes()))
}

/// Writes all the bytes yielded by this `HexToBytesIter` to the provided slice.
/// Writes all the bytes yielded by this `HexToBytesIter` to the provided uninitialized slice.
///
/// Stops writing if this `HexToBytesIter` yields an `InvalidCharError`.
/// Stops writing if this `HexToBytesIter` yields an `InvalidCharError`. On error, bytes
/// written before the error remain initialized in `buf`, but the caller cannot rely on
/// any particular prefix being initialized and must not treat `buf` as `&[u8]`.
///
/// # Panics
///
/// Panics if the length of this `HexToBytesIter` is not equal to the length of the provided
/// slice.
pub(crate) fn drain_to_uninit_slice(
self,
buf: &mut [MaybeUninit<u8>],
) -> Result<(), InvalidCharError> {
// IF YOU CHANGE THIS FUNCTION DO THE ONE BELOW TOO.
assert_eq!(self.len(), buf.len());
let mut ptr = buf.as_mut_ptr().cast::<u8>();
for byte in self {
// SAFETY: for loop iterates `len` times, and `buf` has length `len`.
// Writing through a `*mut u8` derived from `*mut MaybeUninit<u8>` is sound
// because the two have identical layout and `MaybeUninit<u8>` imposes no
// validity requirement on the bytes being overwritten.
unsafe {
core::ptr::write(ptr, byte?);
ptr = ptr.add(1);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 1705412:

Why use ptr::write instead of Maybeuninit::write? That would save you unsafety, casts, and pointer arithmetic.

}
Ok(())
}

// Exactly the same as the function above except without the `MaybeUinit` stuff. Used to test
// the function above.
#[cfg(test)]
pub(crate) fn drain_to_slice(self, buf: &mut [u8]) -> Result<(), InvalidCharError> {
assert_eq!(self.len(), buf.len());
let mut ptr = buf.as_mut_ptr();
Expand Down
32 changes: 29 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ pub mod prelude {

#[cfg(feature = "alloc")]
use alloc::vec::Vec;
use core::mem::MaybeUninit;

pub(crate) use table::Table;

Expand Down Expand Up @@ -137,10 +138,20 @@ pub fn decode_to_vec(hex: &str) -> Result<Vec<u8>, DecodeVariableLengthBytesErro
/// `N * 2`.)
pub fn decode_to_array<const N: usize>(hex: &str) -> Result<[u8; N], DecodeFixedLengthBytesError> {
if hex.len() == N * 2 {
let mut ret = [0u8; N];
// SAFETY: `[MaybeUninit<u8>; N]` has no initialization requirement,
// so an uninitialized array of them is sound. This is the standard
// `uninit_array` pattern.
let mut ret: [MaybeUninit<u8>; N] = unsafe { MaybeUninit::uninit().assume_init() };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 1705412:

Can you find a citation for this being the "standard uninit_array pattern"? It appears intuitively that MaybeUninit::uninit().assume_init() is always unsound.

AFAICT, by reading for example this rust-lang thread, the correct way to do this on stable Rust is [const { MaybeUninit::uninit() }; N].


// checked above
HexToBytesIter::new_unchecked(hex).drain_to_slice(&mut ret)?;
Ok(ret)
HexToBytesIter::new_unchecked(hex).drain_to_uninit_slice(&mut ret)?;

// SAFETY: `drain_to_uninit_slice` returning `Ok` means all N bytes
// were written. `[MaybeUninit<u8>; N]` and `[u8; N]` have identical
// layout, so the transmute is sound.
#[allow(clippy::borrow_as_ptr)]
#[allow(clippy::ptr_as_ptr)]
Ok(unsafe { (&ret as *const _ as *const [u8; N]).read() })
} else {
Err(InvalidLengthError { invalid: hex.len(), expected: 2 * N }.into())
}
Expand Down Expand Up @@ -273,4 +284,19 @@ mod tests {
assert_eq!(HASH[0], 0x00);
assert_eq!(HASH[31], 0x6f);
}

// This implicitly test `drain_to_uninit_slice()`.
// In `iter::hex_to_bytes_slice_drain` we test `drain_to_slice()`
#[test]
fn decode_to_vec() {
let hex = "deadbeef";
let want = [0xde, 0xad, 0xbe, 0xef];
let got = crate::decode_to_vec(hex).unwrap();
assert_eq!(got, want);

let hex = "";
let want: [u8; 0] = [];
let got = crate::decode_to_vec(hex).unwrap();
assert_eq!(got, want);
}
}