Skip to content
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

Update rand_core::Error in line with getrandom::Error #864

Merged
merged 4 commits into from
Aug 28, 2019
Merged
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
12 changes: 7 additions & 5 deletions rand_core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- `alloc` feature in `no_std` is available since Rust 1.36 (#856)

## [0.5.1] - 2019-09-02
### Added
## [0.5.1] - 2019-08-28
- `OsRng` added to `rand_core` (#863)
- `Error::INTERNAL_START` and `Error::CUSTOM_START` constants (#864)
- `Error::raw_os_error` method (#864)
- `Debug` and `Display` formatting for `getrandom` error codes without `std` (#864)
### Changed
- `alloc` feature in `no_std` is available since Rust 1.36 (#856)
- Added `#[inline]` to `Error` conversion methods (#864)

## [0.5.0] - 2019-06-06
### Changed
Expand Down
62 changes: 59 additions & 3 deletions rand_core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use core::num::NonZeroU32;
/// In order to be compatible with `std` and `no_std`, this type has two
/// possible implementations: with `std` a boxed `Error` trait object is stored,
/// while with `no_std` we merely store an error code.
#[derive(Debug)]
pub struct Error {
#[cfg(feature="std")]
inner: Box<dyn std::error::Error + Send + Sync + 'static>,
Expand All @@ -32,6 +31,7 @@ impl Error {
///
/// See also `From<NonZeroU32>`, which is available with and without `std`.
#[cfg(feature="std")]
#[inline]
pub fn new<E>(err: E) -> Self
where E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>
{
Expand All @@ -43,6 +43,7 @@ impl Error {
/// When configured with `std`, this is a trivial operation and never
/// panics. Without `std`, this method is simply unavailable.
#[cfg(feature="std")]
#[inline]
pub fn inner(&self) -> &(dyn std::error::Error + Send + Sync + 'static) {
&*self.inner
}
Expand All @@ -52,15 +53,45 @@ impl Error {
/// When configured with `std`, this is a trivial operation and never
/// panics. Without `std`, this method is simply unavailable.
#[cfg(feature="std")]
#[inline]
pub fn take_inner(self) -> Box<dyn std::error::Error + Send + Sync + 'static> {
self.inner
}

/// Codes below this point represent OS Errors (i.e. positive i32 values).
/// Codes at or above this point, but below [`Error::CUSTOM_START`] are
/// reserved for use by the `rand` and `getrandom` crates.
pub const INTERNAL_START: u32 = 1 << 31;

/// Codes at or above this point can be used by users to define their own
/// custom errors.
pub const CUSTOM_START: u32 = (1 << 31) + (1 << 30);

/// Extract the raw OS error code (if this error came from the OS)
///
/// This method is identical to `std::io::Error::raw_os_error()`, except
/// that it works in `no_std` contexts. If this method returns `None`, the
/// error value can still be formatted via the `Diplay` implementation.
#[inline]
pub fn raw_os_error(&self) -> Option<i32> {
#[cfg(feature="std")] {
if let Some(e) = self.inner.downcast_ref::<std::io::Error>() {
return e.raw_os_error();
}
}
match self.code() {
Some(code) if u32::from(code) < Self::INTERNAL_START =>
Some(u32::from(code) as i32),
_ => None,
}
}

/// Retrieve the error code, if any.
///
/// If this `Error` was constructed via `From<NonZeroU32>`, then this method
/// will return this `NonZeroU32` code (for `no_std` this is always the
/// case). Otherwise, this method will return `None`.
#[inline]
pub fn code(&self) -> Option<NonZeroU32> {
#[cfg(feature="std")] {
self.inner.downcast_ref::<ErrorCode>().map(|c| c.0)
Expand All @@ -71,18 +102,36 @@ impl Error {
}
}

impl fmt::Debug for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
#[cfg(feature="std")] {
write!(f, "Error {{ inner: {:?} }}", self.inner)
}
#[cfg(all(feature="getrandom", not(feature="std")))] {
getrandom::Error::from(self.code).fmt(f)
}
#[cfg(not(feature="getrandom"))] {
Copy link
Member

Choose a reason for hiding this comment

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

You should use cfg(not(any(feature="getrandom", feature="std"))). Same for Display impl. Maybe use cfg_if to be less error-prone?

Copy link
Member Author

Choose a reason for hiding this comment

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

std implies getrandom

write!(f, "Error {{ code: {} }}", self.code)
}
}
}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
#[cfg(feature="std")] {
write!(f, "{}", self.inner)
}
#[cfg(not(feature="std"))] {
#[cfg(all(feature="getrandom", not(feature="std")))] {
getrandom::Error::from(self.code).fmt(f)
}
#[cfg(not(feature="getrandom"))] {
write!(f, "error code {}", self.code)
}
}
}

impl From<NonZeroU32> for Error {
#[inline]
fn from(code: NonZeroU32) -> Self {
#[cfg(feature="std")] {
Error { inner: Box::new(ErrorCode(code)) }
Expand All @@ -95,6 +144,7 @@ impl From<NonZeroU32> for Error {

#[cfg(feature="getrandom")]
impl From<getrandom::Error> for Error {
#[inline]
fn from(error: getrandom::Error) -> Self {
#[cfg(feature="std")] {
Error { inner: Box::new(error) }
Expand All @@ -107,15 +157,21 @@ impl From<getrandom::Error> for Error {

#[cfg(feature="std")]
impl std::error::Error for Error {
#[inline]
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
self.inner.source()
}
}

#[cfg(feature="std")]
impl From<Error> for std::io::Error {
#[inline]
fn from(error: Error) -> Self {
std::io::Error::new(std::io::ErrorKind::Other, error)
if let Some(code) = error.raw_os_error() {
std::io::Error::from_raw_os_error(code)
} else {
std::io::Error::new(std::io::ErrorKind::Other, error)
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions rand_jitter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.2.1] - 2019-08-16
### Changed
- `TimerError` changed to `repr(u32)` (#864)
- `TimerError` enum values all increased by `1<<30` to match new `rand_core::Error` range (#864)

## [0.2.0] - 2019-06-06
- Bump `rand_core` version
- Support new `Error` type in `rand_core` 0.5
Expand Down
2 changes: 1 addition & 1 deletion rand_jitter/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rand_jitter"
version = "0.2.0"
version = "0.2.1"
authors = ["The Rand Project Developers"]
license = "MIT OR Apache-2.0"
readme = "README.md"
Expand Down
16 changes: 10 additions & 6 deletions rand_jitter/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,28 @@
use rand_core::Error;
use core::fmt;

/// Base code for all `JitterRng` errors
const ERROR_BASE: u32 = 0xAE53_0400;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe write it like const ERROR_BASE: u32 = Error::INTERNAL_START + <some const>?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this actually clearer? To me it's not.


/// An error that can occur when [`JitterRng::test_timer`] fails.
///
/// All variants have a value of 0x6e530400 = 1850934272 plus a small
/// All variants have a value of 0xAE530400 = 2924676096 plus a small
/// increment (1 through 5).
///
/// [`JitterRng::test_timer`]: crate::JitterRng::test_timer
#[derive(Debug, Clone, PartialEq, Eq)]
#[repr(u32)]
pub enum TimerError {
/// No timer available.
NoTimer = 0x6e530401,
NoTimer = ERROR_BASE + 1,
/// Timer too coarse to use as an entropy source.
CoarseTimer = 0x6e530402,
CoarseTimer = ERROR_BASE + 2,
/// Timer is not monotonically increasing.
NotMonotonic = 0x6e530403,
NotMonotonic = ERROR_BASE + 3,
/// Variations of deltas of time too small.
TinyVariantions = 0x6e530404,
TinyVariantions = ERROR_BASE + 4,
/// Too many stuck results (indicating no added entropy).
TooManyStuck = 0x6e530405,
TooManyStuck = ERROR_BASE + 5,
#[doc(hidden)]
__Nonexhaustive,
}
Expand Down