From 70397f223e8d97a6df5f8724856bca2cdea79771 Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 10 Apr 2019 16:45:02 +0300 Subject: [PATCH 1/5] rand_core::Error new version draft --- rand_core/Cargo.toml | 2 + rand_core/src/error.rs | 193 ++++++++++------------------------ rand_core/src/lib.rs | 2 +- rand_jitter/src/error.rs | 12 --- rand_os/Cargo.toml | 1 + rand_os/src/lib.rs | 20 ++-- src/lib.rs | 3 +- src/rngs/adapter/read.rs | 12 +-- src/rngs/adapter/reseeding.rs | 19 +--- src/rngs/os.rs | 31 +++++- 10 files changed, 101 insertions(+), 194 deletions(-) diff --git a/rand_core/Cargo.toml b/rand_core/Cargo.toml index 98e47ddf899..67bf8ca462d 100644 --- a/rand_core/Cargo.toml +++ b/rand_core/Cargo.toml @@ -21,7 +21,9 @@ appveyor = { repository = "rust-random/rand" } std = ["alloc"] # use std library; should be default but for above bug alloc = [] # enables Vec and Box support without std serde1 = ["serde", "serde_derive"] # enables serde for BlockRng wrapper +default = ["getrandom", "std"] [dependencies] +getrandom = { version = "0.1", optional = true } serde = { version = "1", optional = true } serde_derive = { version = "^1.0.38", optional = true } diff --git a/rand_core/src/error.rs b/rand_core/src/error.rs index 5a8459ea8be..127fff64b4d 100644 --- a/rand_core/src/error.rs +++ b/rand_core/src/error.rs @@ -9,169 +9,84 @@ //! Error types use core::fmt; +use core::num::NonZeroU32; -#[cfg(feature="std")] -use std::error::Error as stdError; -#[cfg(feature="std")] -use std::io; - -/// Error kind which can be matched over. -#[derive(PartialEq, Eq, Debug, Copy, Clone)] -pub enum ErrorKind { - /// Feature is not available; not recoverable. - /// - /// This is the most permanent failure type and implies the error cannot be - /// resolved simply by retrying (e.g. the feature may not exist in this - /// build of the application or on the current platform). - Unavailable, - /// General failure; there may be a chance of recovery on retry. - /// - /// This is the catch-all kind for errors from known and unknown sources - /// which do not have a more specific kind / handling method. - /// - /// It is suggested to retry a couple of times or retry later when - /// handling; some error sources may be able to resolve themselves, - /// although this is not likely. - Unexpected, - /// A transient failure which likely can be resolved or worked around. - /// - /// This error kind exists for a few specific cases where it is known that - /// the error likely can be resolved internally, but is reported anyway. - Transient, - /// Not ready yet: recommended to try again a little later. - /// - /// This error kind implies the generator needs more time or needs some - /// other part of the application to do something else first before it is - /// ready for use; for example this may be used by external generators - /// which require time for initialization. - NotReady, - #[doc(hidden)] - __Nonexhaustive, -} - -impl ErrorKind { - /// True if this kind of error may resolve itself on retry. - /// - /// See also `should_wait()`. - pub fn should_retry(self) -> bool { - self != ErrorKind::Unavailable - } - - /// True if we should retry but wait before retrying - /// - /// This implies `should_retry()` is true. - pub fn should_wait(self) -> bool { - self == ErrorKind::NotReady - } - - /// A description of this error kind - pub fn description(self) -> &'static str { - match self { - ErrorKind::Unavailable => "permanently unavailable", - ErrorKind::Unexpected => "unexpected failure", - ErrorKind::Transient => "transient failure", - ErrorKind::NotReady => "not ready yet", - ErrorKind::__Nonexhaustive => unreachable!(), - } - } -} +/// TODO +#[derive(Debug, Copy, Clone)] +struct ErrorCode(NonZeroU32); - -/// Error type of random number generators -/// -/// This is a relatively simple error type, designed for compatibility with and -/// without the Rust `std` library. It embeds a "kind" code, a message (static -/// string only), and an optional chained cause (`std` only). The `kind` and -/// `msg` fields can be accessed directly; cause can be accessed via -/// `std::error::Error::cause` or `Error::take_cause`. Construction can only be -/// done via `Error::new` or `Error::with_cause`. +/// TODO #[derive(Debug)] pub struct Error { - /// The error kind - pub kind: ErrorKind, - /// The error message - pub msg: &'static str, + #[cfg(not(feature="std"))] + code: ErrorCode, #[cfg(feature="std")] - cause: Option>, + cause: Box, } impl Error { - /// Create a new instance, with specified kind and a message. - pub fn new(kind: ErrorKind, msg: &'static str) -> Self { + /// TODO + pub fn from_code(code: NonZeroU32) -> Self { + let code = ErrorCode(code); + #[cfg(not(feature="std"))] { + Self { code } + } #[cfg(feature="std")] { - Error { kind, msg, cause: None } + Self { cause: Box::new(code) } } + } + + /// TODO + pub fn code(&self) -> Option { #[cfg(not(feature="std"))] { - Error { kind, msg } + Some(self.code.0) + } + #[cfg(feature="std")] { + self.cause.downcast_ref().map(|r: &ErrorCode| r.0) } - } - - /// Create a new instance, with specified kind, message, and a - /// chained cause. - /// - /// Note: `stdError` is an alias for `std::error::Error`. - /// - /// If not targetting `std` (i.e. `no_std`), this function is replaced by - /// another with the same prototype, except that there are no bounds on the - /// type `E` (because both `Box` and `stdError` are unavailable), and the - /// `cause` is ignored. - #[cfg(feature="std")] - pub fn with_cause(kind: ErrorKind, msg: &'static str, cause: E) -> Self - where E: Into> - { - Error { kind, msg, cause: Some(cause.into()) } - } - - /// Create a new instance, with specified kind, message, and a - /// chained cause. - /// - /// In `no_std` mode the *cause* is ignored. - #[cfg(not(feature="std"))] - pub fn with_cause(kind: ErrorKind, msg: &'static str, _cause: E) -> Self { - Error { kind, msg } - } - - /// Take the cause, if any. This allows the embedded cause to be extracted. - /// This uses `Option::take`, leaving `self` with no cause. - #[cfg(feature="std")] - pub fn take_cause(&mut self) -> Option> { - self.cause.take() } } impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - #[cfg(feature="std")] { - if let Some(ref cause) = self.cause { - return write!(f, "{} ({}); cause: {}", - self.msg, self.kind.description(), cause); - } - } - write!(f, "{} ({})", self.msg, self.kind.description()) + // TODO + Ok(()) } } -#[cfg(feature="std")] -impl stdError for Error { - fn description(&self) -> &str { - self.msg - } - - fn cause(&self) -> Option<&stdError> { - self.cause.as_ref().map(|e| e.as_ref() as &stdError) +impl fmt::Display for ErrorCode { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO + Ok(()) } } #[cfg(feature="std")] -impl From for io::Error { - fn from(error: Error) -> Self { - use std::io::ErrorKind::*; - match error.kind { - ErrorKind::Unavailable => io::Error::new(NotFound, error), - ErrorKind::Unexpected | - ErrorKind::Transient => io::Error::new(Other, error), - ErrorKind::NotReady => io::Error::new(WouldBlock, error), - ErrorKind::__Nonexhaustive => unreachable!(), +mod std_impls { + use std::error::Error as StdError; + use std::io; + use super::{Error, ErrorCode}; + + impl Error { + /// TODO + pub fn from_cause(cause: E) -> Self + where E: Into> + { + Self { cause: cause.into() } + } + + /// TODO + pub fn cause(self) -> Box { + self.cause + } + } + + impl StdError for ErrorCode { } + impl StdError for Error { } + + impl From for io::Error { + fn from(error: Error) -> Self { + io::Error::new(io::ErrorKind::Other, error) } } } diff --git a/rand_core/src/lib.rs b/rand_core/src/lib.rs index 4b0e6e48b01..6d48e892592 100644 --- a/rand_core/src/lib.rs +++ b/rand_core/src/lib.rs @@ -50,7 +50,7 @@ use core::ptr::copy_nonoverlapping; #[cfg(all(feature="alloc", not(feature="std")))] use alloc::boxed::Box; -pub use error::{ErrorKind, Error}; +pub use error::Error; mod error; diff --git a/rand_jitter/src/error.rs b/rand_jitter/src/error.rs index b8bc37f6b38..8e3dabf408a 100644 --- a/rand_jitter/src/error.rs +++ b/rand_jitter/src/error.rs @@ -6,8 +6,6 @@ // , at your // option. This file may not be copied, modified, or distributed // except according to those terms. - -use rand_core::{Error, ErrorKind}; use core::fmt; /// An error that can occur when [`JitterRng::test_timer`] fails. @@ -54,13 +52,3 @@ impl ::std::error::Error for TimerError { self.description() } } - -impl From for Error { - fn from(err: TimerError) -> Error { - // Timer check is already quite permissive of failures so we don't - // expect false-positive failures, i.e. any error is irrecoverable. - Error::with_cause(ErrorKind::Unavailable, - "timer jitter failed basic quality tests", err) - } -} - diff --git a/rand_os/Cargo.toml b/rand_os/Cargo.toml index 402042a9b2d..bb78d129341 100644 --- a/rand_os/Cargo.toml +++ b/rand_os/Cargo.toml @@ -16,6 +16,7 @@ travis-ci = { repository = "rust-random/rand" } appveyor = { repository = "rust-random/rand" } [features] +std = ["rand_core/std", "getrandom/std"] log = ["getrandom/log"] # re-export optional WASM dependencies to avoid breakage: wasm-bindgen = ["getrandom/wasm-bindgen"] diff --git a/rand_os/src/lib.rs b/rand_os/src/lib.rs index 2849f4fa2d8..045003315ae 100644 --- a/rand_os/src/lib.rs +++ b/rand_os/src/lib.rs @@ -28,20 +28,18 @@ #![deny(missing_debug_implementations)] #![doc(test(attr(allow(unused_variables), deny(warnings))))] -#![cfg_attr(feature = "stdweb", recursion_limit="128")] - #![no_std] // but see getrandom crate pub use rand_core; // re-export use getrandom::getrandom; -use rand_core::{CryptoRng, RngCore, Error, ErrorKind, impls}; +use rand_core::{CryptoRng, RngCore, Error, impls}; /// A random number generator that retrieves randomness from from the /// operating system. /// /// This is a zero-sized struct. It can be freely constructed with `OsRng`. -/// +/// /// The implementation is provided by the [getrandom] crate. Refer to /// [getrandom] documentation for details. /// @@ -84,12 +82,14 @@ impl RngCore for OsRng { } fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { - // Some systems do not support reading 0 random bytes. - // (And why waste a system call?) - if dest.len() == 0 { return Ok(()); } - - getrandom(dest).map_err(|e| - Error::with_cause(ErrorKind::Unavailable, "OsRng failed", e)) + getrandom(dest).map_err(|e| { + #[cfg(feature="std")] { + rand_core::Error::from_cause(e) + } + #[cfg(not(feature="std"))] { + rand_core::Error::from_code(e.code()) + } + }) } } diff --git a/src/lib.rs b/src/lib.rs index 3b59f25086d..39f865fca91 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -79,8 +79,7 @@ extern crate rand_pcg; // Re-exports from rand_core -pub use rand_core::{RngCore, CryptoRng, SeedableRng}; -pub use rand_core::{ErrorKind, Error}; +pub use rand_core::{RngCore, CryptoRng, SeedableRng, Error}; // Public exports #[cfg(feature="std")] pub use rngs::thread::thread_rng; diff --git a/src/rngs/adapter/read.rs b/src/rngs/adapter/read.rs index b7787702d6a..e193b0e91b1 100644 --- a/src/rngs/adapter/read.rs +++ b/src/rngs/adapter/read.rs @@ -11,7 +11,7 @@ use std::io::Read; -use rand_core::{RngCore, Error, ErrorKind, impls}; +use rand_core::{RngCore, Error, impls}; /// An RNG that reads random bytes straight from any type supporting @@ -73,15 +73,7 @@ impl RngCore for ReadRng { fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { if dest.len() == 0 { return Ok(()); } // Use `std::io::read_exact`, which retries on `ErrorKind::Interrupted`. - self.reader.read_exact(dest).map_err(|err| { - match err.kind() { - ::std::io::ErrorKind::UnexpectedEof => Error::with_cause( - ErrorKind::Unavailable, - "not enough bytes available, reached end of source", err), - _ => Error::with_cause(ErrorKind::Unavailable, - "error reading from Read source", err) - } - }) + self.reader.read_exact(dest).map_err(Error::from_cause) } } diff --git a/src/rngs/adapter/reseeding.rs b/src/rngs/adapter/reseeding.rs index a1ebe78f739..a237190e7a4 100644 --- a/src/rngs/adapter/reseeding.rs +++ b/src/rngs/adapter/reseeding.rs @@ -12,7 +12,7 @@ use core::mem::size_of; -use rand_core::{RngCore, CryptoRng, SeedableRng, Error, ErrorKind}; +use rand_core::{RngCore, CryptoRng, SeedableRng, Error}; use rand_core::block::{BlockRngCore, BlockRng}; /// A wrapper around any PRNG that implements [`BlockRngCore`], that adds the @@ -240,21 +240,10 @@ where R: BlockRngCore + SeedableRng, let num_bytes = results.as_ref().len() * size_of::<::Item>(); - let threshold = if let Err(e) = self.reseed() { - let delay = match e.kind { - ErrorKind::Transient => num_bytes as i64, - kind @ _ if kind.should_retry() => self.threshold >> 8, - _ => self.threshold, - }; - warn!("Reseeding RNG delayed reseeding by {} bytes due to \ - error from source: {}", delay, e); - delay - } else { - self.fork_counter = global_fork_counter; - self.threshold - }; + self.reseed().unwrap(); + self.fork_counter = global_fork_counter; - self.bytes_until_reseed = threshold - num_bytes as i64; + self.bytes_until_reseed = self.threshold - num_bytes as i64; self.inner.generate(results); } } diff --git a/src/rngs/os.rs b/src/rngs/os.rs index e71ea80498c..1ee590de0c7 100644 --- a/src/rngs/os.rs +++ b/src/rngs/os.rs @@ -7,20 +7,29 @@ // except according to those terms. //! Interface to the random number generator of the operating system. +// Note: keep this code in sync with `rand_os` crate! use getrandom::getrandom; -use rand_core::{CryptoRng, RngCore, Error, ErrorKind, impls}; +use rand_core::{CryptoRng, RngCore, Error, impls}; /// A random number generator that retrieves randomness from from the /// operating system. /// /// This is a zero-sized struct. It can be freely constructed with `OsRng`. -/// +/// /// The implementation is provided by the [getrandom] crate. Refer to /// [getrandom] documentation for details. /// -/// ## Example +/// # Usage example +/// ``` +/// use rand::{Rng, rngs::OsRng}; +/// +/// let mut key = [0u8; 16]; +/// OsRng.fill_bytes(&mut key); +/// let random_u64 = OsRng.next_u64(); +/// ``` /// +/// PRNG initialization: /// ``` /// use rand::rngs::{StdRng, OsRng}; /// use rand::{RngCore, SeedableRng}; @@ -60,8 +69,14 @@ impl RngCore for OsRng { } fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { - getrandom(dest).map_err(|e| - Error::with_cause(ErrorKind::Unavailable, "OsRng failed", e)) + getrandom(dest).map_err(|e| { + #[cfg(feature="std")] { + rand_core::Error::from_cause(e) + } + #[cfg(not(feature="std"))] { + rand_core::Error::from_code(e.code()) + } + }) } } @@ -72,3 +87,9 @@ fn test_os_rng() { assert!(x != 0); assert!(x != y); } + +#[test] +fn test_construction() { + let mut rng = OsRng::default(); + assert!(rng.next_u64() != 0); +} From 91aba6d4b36885709e22921b6775afb0d8c4e34b Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 10 Apr 2019 16:46:34 +0300 Subject: [PATCH 2/5] remove default features --- rand_core/Cargo.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/rand_core/Cargo.toml b/rand_core/Cargo.toml index 67bf8ca462d..98e47ddf899 100644 --- a/rand_core/Cargo.toml +++ b/rand_core/Cargo.toml @@ -21,9 +21,7 @@ appveyor = { repository = "rust-random/rand" } std = ["alloc"] # use std library; should be default but for above bug alloc = [] # enables Vec and Box support without std serde1 = ["serde", "serde_derive"] # enables serde for BlockRng wrapper -default = ["getrandom", "std"] [dependencies] -getrandom = { version = "0.1", optional = true } serde = { version = "1", optional = true } serde_derive = { version = "^1.0.38", optional = true } From 00e34993d138bdb3ef4a2529b35878c139598a3a Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 10 Apr 2019 17:08:53 +0300 Subject: [PATCH 3/5] fix read test --- src/rngs/adapter/read.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/rngs/adapter/read.rs b/src/rngs/adapter/read.rs index e193b0e91b1..c2c64ea9544 100644 --- a/src/rngs/adapter/read.rs +++ b/src/rngs/adapter/read.rs @@ -80,7 +80,8 @@ impl RngCore for ReadRng { #[cfg(test)] mod test { use super::ReadRng; - use {RngCore, ErrorKind}; + use RngCore; + use std::io; #[test] fn test_reader_rng_u64() { @@ -123,6 +124,13 @@ mod test { let mut rng = ReadRng::new(&v[..]); - assert!(rng.try_fill_bytes(&mut w).err().unwrap().kind == ErrorKind::Unavailable); + let err_kind = rng.try_fill_bytes(&mut w) + .err() + .unwrap() + .cause() + .downcast_ref() + .map(|e: &io::Error| e.kind()) + .unwrap(); + assert_eq!(err_kind, io::ErrorKind::UnexpectedEof); } } From c269d5316d1d1cc4876583498bc4144c79bd4b88 Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 10 Apr 2019 17:21:10 +0300 Subject: [PATCH 4/5] fix doctest --- src/rngs/os.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rngs/os.rs b/src/rngs/os.rs index 1ee590de0c7..a9a15a5f3b6 100644 --- a/src/rngs/os.rs +++ b/src/rngs/os.rs @@ -22,7 +22,7 @@ use rand_core::{CryptoRng, RngCore, Error, impls}; /// /// # Usage example /// ``` -/// use rand::{Rng, rngs::OsRng}; +/// use rand::{RngCore, rngs::OsRng}; /// /// let mut key = [0u8; 16]; /// OsRng.fill_bytes(&mut key); From 461a1e9a0d648cdeba729e1dbd4c6fe9109b8b65 Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 10 Apr 2019 17:39:24 +0300 Subject: [PATCH 5/5] fix rand_jitter doctest --- rand_jitter/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rand_jitter/src/lib.rs b/rand_jitter/src/lib.rs index 338a6b966a1..727ea8e5d55 100644 --- a/rand_jitter/src/lib.rs +++ b/rand_jitter/src/lib.rs @@ -217,10 +217,11 @@ impl JitterRng { /// # Example /// /// ``` - /// # use rand_jitter::rand_core::{RngCore, Error}; + /// # use rand_jitter::rand_core::RngCore; + /// # use rand_jitter::TimerError; /// use rand_jitter::JitterRng; /// - /// # fn try_inner() -> Result<(), Error> { + /// # fn try_inner() -> Result<(), TimerError> { /// fn get_nstime() -> u64 { /// use std::time::{SystemTime, UNIX_EPOCH}; ///