-
-
Notifications
You must be signed in to change notification settings - Fork 468
Implement fill_via_chunks
without using zerocopy or unsafe
.
#1575
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
Changes from all commits
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 |
---|---|---|
|
@@ -18,8 +18,6 @@ | |
//! non-reproducible sources (e.g. `OsRng`) need not bother with it. | ||
|
||
use crate::RngCore; | ||
use core::cmp::min; | ||
use zerocopy::{Immutable, IntoBytes}; | ||
|
||
/// Implement `next_u64` via `next_u32`, little-endian order. | ||
pub fn next_u64_via_u32<R: RngCore + ?Sized>(rng: &mut R) -> u64 { | ||
|
@@ -53,17 +51,22 @@ pub fn fill_bytes_via_next<R: RngCore + ?Sized>(rng: &mut R, dest: &mut [u8]) { | |
} | ||
} | ||
|
||
trait Observable: IntoBytes + Immutable + Copy { | ||
fn to_le(self) -> Self; | ||
trait Observable: Copy { | ||
type Bytes: Sized + AsRef<[u8]>; | ||
fn to_le_bytes(self) -> Self::Bytes; | ||
} | ||
impl Observable for u32 { | ||
fn to_le(self) -> Self { | ||
self.to_le() | ||
type Bytes = [u8; 4]; | ||
|
||
fn to_le_bytes(self) -> Self::Bytes { | ||
Self::to_le_bytes(self) | ||
} | ||
} | ||
impl Observable for u64 { | ||
fn to_le(self) -> Self { | ||
self.to_le() | ||
type Bytes = [u8; 8]; | ||
|
||
fn to_le_bytes(self) -> Self::Bytes { | ||
Self::to_le_bytes(self) | ||
} | ||
} | ||
|
||
|
@@ -72,32 +75,35 @@ impl Observable for u64 { | |
/// Returns `(n, byte_len)`. `src[..n]` is consumed (and possibly mutated), | ||
/// `dest[..byte_len]` is filled. `src[n..]` and `dest[byte_len..]` are left | ||
/// unaltered. | ||
fn fill_via_chunks<T: Observable>(src: &mut [T], dest: &mut [u8]) -> (usize, usize) { | ||
fn fill_via_chunks<T: Observable>(src: &[T], dest: &mut [u8]) -> (usize, usize) { | ||
let size = core::mem::size_of::<T>(); | ||
let byte_len = min(core::mem::size_of_val(src), dest.len()); | ||
let num_chunks = (byte_len + size - 1) / size; | ||
|
||
// Byte-swap for portability of results. This must happen before copying | ||
// since the size of dest is not guaranteed to be a multiple of T or to be | ||
// sufficiently aligned. | ||
if cfg!(target_endian = "big") { | ||
for x in &mut src[..num_chunks] { | ||
*x = x.to_le(); | ||
} | ||
} | ||
|
||
dest[..byte_len].copy_from_slice(&<[T]>::as_bytes(&src[..num_chunks])[..byte_len]); | ||
// Always use little endian for portability of results. | ||
|
||
(num_chunks, byte_len) | ||
let mut dest = dest.chunks_exact_mut(size); | ||
let mut src = src.iter(); | ||
|
||
let zipped = dest.by_ref().zip(src.by_ref()); | ||
let num_chunks = zipped.len(); | ||
zipped.for_each(|(dest, src)| dest.copy_from_slice(src.to_le_bytes().as_ref())); | ||
|
||
let byte_len = num_chunks * size; | ||
if let (dest_tail @ [_, ..], Some(src)) = (dest.into_remainder(), src.next()) { | ||
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. Nit: I find the if let Some(src) = src.next() {
// We have consumed all full chunks of dest, but not src.
let dest = dest.into_remainder();
let n = dest.len();
if n > 0 {
dest.copy_from_slice(&src.to_le_bytes().as_ref()[..n]);
return (num_chunks + 1, byte_len + n)
}
}
(num_chunks, byte_len) 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. The usage of a slice match pattern to check for a non-empty tail is not so obvious, no. This alternative compiles to identical assembly so I'll take it by preference. |
||
let n = dest_tail.len(); | ||
dest_tail.copy_from_slice(&src.to_le_bytes().as_ref()[..n]); | ||
(num_chunks + 1, byte_len + n) | ||
} else { | ||
(num_chunks, byte_len) | ||
} | ||
} | ||
|
||
/// Implement `fill_bytes` by reading chunks from the output buffer of a block | ||
/// based RNG. | ||
/// | ||
/// The return values are `(consumed_u32, filled_u8)`. | ||
/// | ||
/// On big-endian systems, endianness of `src[..consumed_u32]` values is | ||
/// swapped. No other adjustments to `src` are made. | ||
/// `src` is not modified; it is taken as a `&mut` reference for backward | ||
/// compatibility with previous versions that did change it. | ||
/// | ||
/// `filled_u8` is the number of filled bytes in `dest`, which may be less than | ||
/// the length of `dest`. | ||
|
@@ -125,6 +131,7 @@ fn fill_via_chunks<T: Observable>(src: &mut [T], dest: &mut [u8]) -> (usize, usi | |
/// } | ||
/// ``` | ||
pub fn fill_via_u32_chunks(src: &mut [u32], dest: &mut [u8]) -> (usize, usize) { | ||
// TODO(SemVer): src: `&[u32]` as we don't mutate it. | ||
fill_via_chunks(src, dest) | ||
} | ||
Comment on lines
133
to
136
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. Does anyone know why this method is public at all? It is used in the impl of I suggest that we deprecate these two methods. 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 was wondering that myself 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. My guess is that it was intended to be used by other crates implementing |
||
|
||
|
@@ -133,8 +140,8 @@ pub fn fill_via_u32_chunks(src: &mut [u32], dest: &mut [u8]) -> (usize, usize) { | |
/// | ||
/// The return values are `(consumed_u64, filled_u8)`. | ||
/// | ||
/// On big-endian systems, endianness of `src[..consumed_u64]` values is | ||
/// swapped. No other adjustments to `src` are made. | ||
/// `src` is not modified; it is taken as a `&mut` reference for backward | ||
/// compatibility with previous versions that did change it. | ||
/// | ||
/// `filled_u8` is the number of filled bytes in `dest`, which may be less than | ||
/// the length of `dest`. | ||
|
@@ -143,6 +150,7 @@ pub fn fill_via_u32_chunks(src: &mut [u32], dest: &mut [u8]) -> (usize, usize) { | |
/// | ||
/// See `fill_via_u32_chunks` for an example. | ||
pub fn fill_via_u64_chunks(src: &mut [u64], dest: &mut [u8]) -> (usize, usize) { | ||
// TODO(SemVer): src: `&[u64]` as we don't mutate it. | ||
fill_via_chunks(src, dest) | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.