From a40f3051605b905be87cb85f344199832580b265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 3 Mar 2025 16:43:29 +0300 Subject: [PATCH 01/17] Tweak `Error` representation --- src/backends/hermit.rs | 6 ++-- src/backends/linux_raw.rs | 5 +-- src/backends/solid.rs | 4 +-- src/backends/wasi_p1.rs | 7 +--- src/error.rs | 71 +++++++++++++++++++++++++-------------- src/util_libc.rs | 14 ++++---- 6 files changed, 59 insertions(+), 48 deletions(-) diff --git a/src/backends/hermit.rs b/src/backends/hermit.rs index 0c0e3a6d..43bf5ff7 100644 --- a/src/backends/hermit.rs +++ b/src/backends/hermit.rs @@ -44,10 +44,8 @@ pub fn fill_inner(mut dest: &mut [MaybeUninit]) -> Result<(), Error> { dest = dest.get_mut(len..).ok_or(Error::UNEXPECTED)?; } code => { - let err = u32::try_from(code.unsigned_abs()) - .ok() - .map_or(Error::UNEXPECTED, Error::from_os_error); - return Err(err); + let code = i32::try_from(code).map_err(|_| Error::UNEXPECTED)?; + return Err(Error::from_os_error(code)); } } } diff --git a/src/backends/linux_raw.rs b/src/backends/linux_raw.rs index a601f07b..953a6584 100644 --- a/src/backends/linux_raw.rs +++ b/src/backends/linux_raw.rs @@ -128,10 +128,7 @@ pub fn fill_inner(mut dest: &mut [MaybeUninit]) -> Result<(), Error> { } Err(_) if ret == EINTR => continue, Err(_) => { - let code: u32 = ret - .wrapping_neg() - .try_into() - .map_err(|_| Error::UNEXPECTED)?; + let code = i32::try_from(ret).map_err(|_| Error::UNEXPECTED)?; return Err(Error::from_os_error(code)); } } diff --git a/src/backends/solid.rs b/src/backends/solid.rs index c4222a7f..467e3db2 100644 --- a/src/backends/solid.rs +++ b/src/backends/solid.rs @@ -14,8 +14,6 @@ pub fn fill_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { if ret >= 0 { Ok(()) } else { - // ITRON error numbers are always negative, so we negate it so that it - // falls in the dedicated OS error range (1..INTERNAL_START). - Err(Error::from_os_error(ret.unsigned_abs())) + Err(Error::from_os_error(ret)) } } diff --git a/src/backends/wasi_p1.rs b/src/backends/wasi_p1.rs index 424b7f96..7a29a979 100644 --- a/src/backends/wasi_p1.rs +++ b/src/backends/wasi_p1.rs @@ -21,11 +21,6 @@ pub fn fill_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { let ret = unsafe { random_get(dest.as_mut_ptr() as i32, dest.len() as i32) }; match ret { 0 => Ok(()), - code => { - let err = u32::try_from(code) - .map(Error::from_os_error) - .unwrap_or(Error::UNEXPECTED); - Err(err) - } + code => Err(Error::from_os_error(code)), } } diff --git a/src/error.rs b/src/error.rs index ccbe7837..0f9a4a32 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,15 +1,18 @@ #[cfg(feature = "std")] extern crate std; -use core::{fmt, num::NonZeroU32}; +use core::fmt; // This private alias mirrors `std::io::RawOsError`: // https://doc.rust-lang.org/std/io/type.RawOsError.html) cfg_if::cfg_if!( if #[cfg(target_os = "uefi")] { type RawOsError = usize; + type NonZeroRawOsError = core::num::NonZeroUsize; + const UEFI_ERROR_FLAG: RawOsError = 1 << (RawOsError::BITS - 1); } else { type RawOsError = i32; + type NonZeroRawOsError = core::num::NonZeroI32; } ); @@ -27,8 +30,10 @@ cfg_if::cfg_if!( /// [`std::error::Error`](https://doc.rust-lang.org/std/error/trait.Error.html) /// - [`std::io::Error`](https://doc.rust-lang.org/std/io/struct.Error.html) implements /// [`From`](https://doc.rust-lang.org/std/convert/trait.From.html). +// note: on non-UEFI targets OS errors are represented as negative integers, +// while on UEFI targets OS errors have the highest bit set to 1. #[derive(Copy, Clone, Eq, PartialEq)] -pub struct Error(NonZeroU32); +pub struct Error(NonZeroRawOsError); impl Error { /// This target/platform is not supported by `getrandom`. @@ -38,28 +43,33 @@ impl Error { /// Encountered an unexpected situation which should not happen in practice. pub const UNEXPECTED: Error = Self::new_internal(2); - /// 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. + /// Deprecated. + #[deprecated] pub const INTERNAL_START: u32 = 1 << 31; - /// Codes at or above this point can be used by users to define their own - /// custom errors. + /// Deprecated. + #[deprecated] pub const CUSTOM_START: u32 = (1 << 31) + (1 << 30); + /// Internal errors can be in the range of 2^16..2^17 + const INTERNAL_START2: RawOsError = 1 << 16; + /// Custom errors can be in the range of 2^17..(2^17 + 2^16) + const CUSTOM_START2: RawOsError = 1 << 17; + /// Creates a new instance of an `Error` from a particular OS error code. /// /// This method is analogous to [`std::io::Error::from_raw_os_error()`][1], /// except that it works in `no_std` contexts and `code` will be - /// replaced with `Error::UNEXPECTED` if it isn't in the range - /// `1..Error::INTERNAL_START`. Thus, for the result `r`, - /// `r == Self::UNEXPECTED || r.raw_os_error().unsigned_abs() == code`. + /// replaced with `Error::UNEXPECTED` in unexpected cases. /// /// [1]: https://doc.rust-lang.org/std/io/struct.Error.html#method.from_raw_os_error #[allow(dead_code)] - pub(super) fn from_os_error(code: u32) -> Self { - match NonZeroU32::new(code) { - Some(code) if code.get() < Self::INTERNAL_START => Self(code), + pub(super) fn from_os_error(code: RawOsError) -> Self { + match NonZeroRawOsError::new(code) { + #[cfg(target_os = "uefi")] + Some(code) if code.get() & UEFI_ERROR_FLAG != 0 => Self(code), + #[cfg(not(target_os = "uefi"))] + Some(code) if code.get() < 0 => Self(code), _ => Self::UNEXPECTED, } } @@ -79,27 +89,38 @@ impl Error { #[inline] pub fn raw_os_error(self) -> Option { let code = self.0.get(); - if code >= Self::INTERNAL_START { - return None; + #[cfg(target_os = "uefi")] + { + if code & UEFI_ERROR_FLAG != 0 { + Some(code) + } else { + None + } + } + + #[cfg(not(target_os = "uefi"))] + { + if code >= 0 { + return None; + } + #[cfg(not(target_os = "solid_asp3"))] + let code = code.checked_neg()?; + Some(code) } - let errno = RawOsError::try_from(code).ok()?; - #[cfg(target_os = "solid_asp3")] - let errno = -errno; - Some(errno) } /// Creates a new instance of an `Error` from a particular custom error code. pub const fn new_custom(n: u16) -> Error { - // SAFETY: code > 0 as CUSTOM_START > 0 and adding n won't overflow a u32. - let code = Error::CUSTOM_START + (n as u32); - Error(unsafe { NonZeroU32::new_unchecked(code) }) + // SAFETY: code > 0 as CUSTOM_START > 0 and adding `n` won't overflow `RawOsError`. + let code = Error::CUSTOM_START2 + (n as RawOsError); + Error(unsafe { NonZeroRawOsError::new_unchecked(code) }) } /// Creates a new instance of an `Error` from a particular internal error code. pub(crate) const fn new_internal(n: u16) -> Error { - // SAFETY: code > 0 as INTERNAL_START > 0 and adding n won't overflow a u32. - let code = Error::INTERNAL_START + (n as u32); - Error(unsafe { NonZeroU32::new_unchecked(code) }) + // SAFETY: code > 0 as INTERNAL_START > 0 and adding `n` won't overflow `RawOsError`. + let code = Error::INTERNAL_START2 + (n as RawOsError); + Error(unsafe { NonZeroRawOsError::new_unchecked(code) }) } fn internal_desc(&self) -> Option<&'static str> { diff --git a/src/util_libc.rs b/src/util_libc.rs index 2f618595..711370b4 100644 --- a/src/util_libc.rs +++ b/src/util_libc.rs @@ -34,14 +34,16 @@ cfg_if! { } pub(crate) fn last_os_error() -> Error { + // We assum that on all targets which use this function `c_int` is equal to `i32` let errno: libc::c_int = unsafe { get_errno() }; - // c_int-to-u32 conversion is lossless for nonnegative values if they are the same size. - const _: () = assert!(core::mem::size_of::() == core::mem::size_of::()); - - match u32::try_from(errno) { - Ok(code) if code != 0 => Error::from_os_error(code), - _ => Error::ERRNO_NOT_POSITIVE, + if errno > 0 { + let code = errno + .checked_neg() + .expect("Positive number can be always negated"); + Error::from_os_error(code) + } else { + Error::ERRNO_NOT_POSITIVE } } From f84c62ee27dedf00437a418869f04bc86d1a1d84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 3 Mar 2025 16:48:54 +0300 Subject: [PATCH 02/17] Tweak note in `last_os_error` --- src/util_libc.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util_libc.rs b/src/util_libc.rs index 711370b4..5ec07ee8 100644 --- a/src/util_libc.rs +++ b/src/util_libc.rs @@ -34,8 +34,8 @@ cfg_if! { } pub(crate) fn last_os_error() -> Error { - // We assum that on all targets which use this function `c_int` is equal to `i32` - let errno: libc::c_int = unsafe { get_errno() }; + // We assume that on all targets which use the `util_libc` module `c_int` is equal to `i32` + let errno: i32 = unsafe { get_errno() }; if errno > 0 { let code = errno From 04872360a9f1107e8aa7e7939424070576d7064b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 3 Mar 2025 16:51:06 +0300 Subject: [PATCH 03/17] Update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e5af794..f4c5c1b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Do not use `dlsym` on MUSL targets in the `linux_android_with_fallback` backend [#602] - Remove `linux_android.rs` and use `getrandom.rs` instead [#603] - Always use `RtlGenRandom` on Windows targets when compiling with pre-1.78 Rust [#610] +- Internal representation of the `Error` type [#614] + +### Deprecated +- `Error::INTERNAL_START` and `Error::CUSTOM_START` associated constants [#614] [#570]: https://github.com/rust-random/getrandom/pull/570 [#572]: https://github.com/rust-random/getrandom/pull/572 @@ -33,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#603]: https://github.com/rust-random/getrandom/pull/603 [#605]: https://github.com/rust-random/getrandom/pull/605 [#610]: https://github.com/rust-random/getrandom/pull/610 +[#614]: https://github.com/rust-random/getrandom/pull/614 ## [0.3.1] - 2025-01-28 From cd61b0b83ed66c2d87c47bdb934d469fa2ccde93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 3 Mar 2025 16:57:12 +0300 Subject: [PATCH 04/17] Add link to the UEFI spec --- src/error.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/error.rs b/src/error.rs index 0f9a4a32..07f48a26 100644 --- a/src/error.rs +++ b/src/error.rs @@ -7,6 +7,8 @@ use core::fmt; // https://doc.rust-lang.org/std/io/type.RawOsError.html) cfg_if::cfg_if!( if #[cfg(target_os = "uefi")] { + // See the UEFI spec for more information: + // https://uefi.org/specs/UEFI/2.10/Apx_D_Status_Codes.html type RawOsError = usize; type NonZeroRawOsError = core::num::NonZeroUsize; const UEFI_ERROR_FLAG: RawOsError = 1 << (RawOsError::BITS - 1); From 8102dd5c2e5b112250e66dd56b85927037cc49d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 3 Mar 2025 17:05:10 +0300 Subject: [PATCH 05/17] Remove `Error` size test --- src/error.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/error.rs b/src/error.rs index 07f48a26..e27e28ca 100644 --- a/src/error.rs +++ b/src/error.rs @@ -199,15 +199,3 @@ impl fmt::Display for Error { } } } - -#[cfg(test)] -mod tests { - use super::Error; - use core::mem::size_of; - - #[test] - fn test_size() { - assert_eq!(size_of::(), 4); - assert_eq!(size_of::>(), 4); - } -} From 2dde6195bee3459cca2d7973cf83f8b88a1ca597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Mon, 3 Mar 2025 19:26:04 +0300 Subject: [PATCH 06/17] Hide deprecated constants --- src/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index e27e28ca..5c33f9f2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -45,12 +45,12 @@ impl Error { /// Encountered an unexpected situation which should not happen in practice. pub const UNEXPECTED: Error = Self::new_internal(2); - /// Deprecated. #[deprecated] + #[doc(hidden)] pub const INTERNAL_START: u32 = 1 << 31; - /// Deprecated. #[deprecated] + #[doc(hidden)] pub const CUSTOM_START: u32 = (1 << 31) + (1 << 30); /// Internal errors can be in the range of 2^16..2^17 From 84e9d58d5d765e8929bc40dc68ebae5ce6c49354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Wed, 5 Mar 2025 17:32:52 +0300 Subject: [PATCH 07/17] Tweak handling of positive/negative code --- src/error.rs | 51 ++++++++++++++++++++++++++++++++++++++---------- src/util_libc.rs | 5 +---- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/error.rs b/src/error.rs index 5c33f9f2..609ad502 100644 --- a/src/error.rs +++ b/src/error.rs @@ -15,6 +15,19 @@ cfg_if::cfg_if!( } else { type RawOsError = i32; type NonZeroRawOsError = core::num::NonZeroI32; + + /// Constant which defines whether the target uses negative or positive codes for + /// system errors. + /// + /// In general, only `libc`-based targets with its infamous `errno` use positive error codes. + /// + /// This constant must cover all backends which rely on `Error::from_os_error`. + const NEGATIVE_OS_CODES: bool = cfg!(any( + target_os = "solid_asp3", + target_os = "hermit", + getrandom_backend = "linux_raw", + all(target_arch = "wasm32", target_os = "wasi", target_env = "p1"), + )); } ); @@ -67,13 +80,29 @@ impl Error { /// [1]: https://doc.rust-lang.org/std/io/struct.Error.html#method.from_raw_os_error #[allow(dead_code)] pub(super) fn from_os_error(code: RawOsError) -> Self { - match NonZeroRawOsError::new(code) { - #[cfg(target_os = "uefi")] - Some(code) if code.get() & UEFI_ERROR_FLAG != 0 => Self(code), - #[cfg(not(target_os = "uefi"))] - Some(code) if code.get() < 0 => Self(code), - _ => Self::UNEXPECTED, + #[cfg(target_os = "uefi")] + if code & UEFI_ERROR_FLAG != 0 { + let code = NonZeroRawOsError::new(code).expect("The highest bit is set to one"); + return Self(code); + } + + #[cfg(not(target_os = "uefi"))] + { + if NEGATIVE_OS_CODES && (code < 0) { + let code = NonZeroRawOsError::new(code).expect("The code is not equal to zero"); + return Self(code); + } + + if !NEGATIVE_OS_CODES && (code > 0) { + let code = code + .checked_neg() + .expect("Positive numbers can be always negated"); + let code = NonZeroRawOsError::new(code).expect("The code is not equal to zero"); + return Self(code); + } } + + Self::UNEXPECTED } /// Extract the raw OS error code (if this error came from the OS) @@ -91,6 +120,7 @@ impl Error { #[inline] pub fn raw_os_error(self) -> Option { let code = self.0.get(); + #[cfg(target_os = "uefi")] { if code & UEFI_ERROR_FLAG != 0 { @@ -103,11 +133,12 @@ impl Error { #[cfg(not(target_os = "uefi"))] { if code >= 0 { - return None; + None + } else if NEGATIVE_OS_CODES { + Some(code) + } else { + code.checked_neg() } - #[cfg(not(target_os = "solid_asp3"))] - let code = code.checked_neg()?; - Some(code) } } diff --git a/src/util_libc.rs b/src/util_libc.rs index 5ec07ee8..a2658c90 100644 --- a/src/util_libc.rs +++ b/src/util_libc.rs @@ -38,10 +38,7 @@ pub(crate) fn last_os_error() -> Error { let errno: i32 = unsafe { get_errno() }; if errno > 0 { - let code = errno - .checked_neg() - .expect("Positive number can be always negated"); - Error::from_os_error(code) + Error::from_os_error(errno) } else { Error::ERRNO_NOT_POSITIVE } From 1ee26121e04d02253e06e466ddca690123b7bf41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Wed, 5 Mar 2025 18:16:29 +0300 Subject: [PATCH 08/17] Tweak handling of positive/negative codes --- src/error.rs | 56 ++++++++++++++++-------------------------------- src/util_libc.rs | 5 ++++- 2 files changed, 22 insertions(+), 39 deletions(-) diff --git a/src/error.rs b/src/error.rs index 609ad502..6d1cdb4f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -15,19 +15,6 @@ cfg_if::cfg_if!( } else { type RawOsError = i32; type NonZeroRawOsError = core::num::NonZeroI32; - - /// Constant which defines whether the target uses negative or positive codes for - /// system errors. - /// - /// In general, only `libc`-based targets with its infamous `errno` use positive error codes. - /// - /// This constant must cover all backends which rely on `Error::from_os_error`. - const NEGATIVE_OS_CODES: bool = cfg!(any( - target_os = "solid_asp3", - target_os = "hermit", - getrandom_backend = "linux_raw", - all(target_arch = "wasm32", target_os = "wasi", target_env = "p1"), - )); } ); @@ -80,29 +67,13 @@ impl Error { /// [1]: https://doc.rust-lang.org/std/io/struct.Error.html#method.from_raw_os_error #[allow(dead_code)] pub(super) fn from_os_error(code: RawOsError) -> Self { - #[cfg(target_os = "uefi")] - if code & UEFI_ERROR_FLAG != 0 { - let code = NonZeroRawOsError::new(code).expect("The highest bit is set to one"); - return Self(code); - } - - #[cfg(not(target_os = "uefi"))] - { - if NEGATIVE_OS_CODES && (code < 0) { - let code = NonZeroRawOsError::new(code).expect("The code is not equal to zero"); - return Self(code); - } - - if !NEGATIVE_OS_CODES && (code > 0) { - let code = code - .checked_neg() - .expect("Positive numbers can be always negated"); - let code = NonZeroRawOsError::new(code).expect("The code is not equal to zero"); - return Self(code); - } + match NonZeroRawOsError::new(code) { + #[cfg(target_os = "uefi")] + Some(code) if code.get() & UEFI_ERROR_FLAG != 0 => Self(code), + #[cfg(not(target_os = "uefi"))] + Some(code) if code.get() < 0 => Self(code), + _ => Self::UNEXPECTED, } - - Self::UNEXPECTED } /// Extract the raw OS error code (if this error came from the OS) @@ -132,12 +103,21 @@ impl Error { #[cfg(not(target_os = "uefi"))] { + // On most targets `std` expects positive error codes while retrieving error strings: + // - `libc`-based targets use `strerror_r` which expects positive error codes. + // - Hermit defers to the `hermit-abi` crate and it expects positive error codes: + // https://docs.rs/hermit-abi/0.4.0/src/hermit_abi/errno.rs.html#400-532 + // - WASIp1 uses the same conventions as `libc`: + // https://github.com/rust-lang/rust/blob/1.85.0/library/std/src/sys/pal/wasi/os.rs#L57-L67 + // + // The only excpetion is Solid, `std` expects negative system error codes, see: + // https://github.com/rust-lang/rust/blob/1.85.0/library/std/src/sys/pal/solid/error.rs#L5-L31 if code >= 0 { None - } else if NEGATIVE_OS_CODES { - Some(code) - } else { + } else if cfg!(not(target_os = "solid_asp3")) { code.checked_neg() + } else { + Some(code) } } } diff --git a/src/util_libc.rs b/src/util_libc.rs index a2658c90..5ec07ee8 100644 --- a/src/util_libc.rs +++ b/src/util_libc.rs @@ -38,7 +38,10 @@ pub(crate) fn last_os_error() -> Error { let errno: i32 = unsafe { get_errno() }; if errno > 0 { - Error::from_os_error(errno) + let code = errno + .checked_neg() + .expect("Positive number can be always negated"); + Error::from_os_error(code) } else { Error::ERRNO_NOT_POSITIVE } From dd3b3e2ad70c48a6af43013d7528d8af79fcfc63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Wed, 5 Mar 2025 18:28:13 +0300 Subject: [PATCH 09/17] Tweak comments in `Error::raw_os_error` --- src/error.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index 6d1cdb4f..f9946147 100644 --- a/src/error.rs +++ b/src/error.rs @@ -92,6 +92,9 @@ impl Error { pub fn raw_os_error(self) -> Option { let code = self.0.get(); + // note: in this method we need to cover only backends which rely on `Error::from_os_error`, + // on all other backends this method always returns `None`. + #[cfg(target_os = "uefi")] { if code & UEFI_ERROR_FLAG != 0 { @@ -105,12 +108,12 @@ impl Error { { // On most targets `std` expects positive error codes while retrieving error strings: // - `libc`-based targets use `strerror_r` which expects positive error codes. - // - Hermit defers to the `hermit-abi` crate and it expects positive error codes: + // - Hermit relies on the `hermit-abi` crate, which expects positive error codes: // https://docs.rs/hermit-abi/0.4.0/src/hermit_abi/errno.rs.html#400-532 // - WASIp1 uses the same conventions as `libc`: // https://github.com/rust-lang/rust/blob/1.85.0/library/std/src/sys/pal/wasi/os.rs#L57-L67 // - // The only excpetion is Solid, `std` expects negative system error codes, see: + // The only exception is Solid, `std` expects negative system error codes, see: // https://github.com/rust-lang/rust/blob/1.85.0/library/std/src/sys/pal/solid/error.rs#L5-L31 if code >= 0 { None From a39698d5d30bcd170dd741a6f5e50251922129d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Wed, 5 Mar 2025 19:45:57 +0300 Subject: [PATCH 10/17] Mark `Error::INTERNAL_START` and `Error::CUSTOM_START` as private --- CHANGELOG.md | 2 +- src/error.rs | 16 ++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4c5c1b7..666c028d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Always use `RtlGenRandom` on Windows targets when compiling with pre-1.78 Rust [#610] - Internal representation of the `Error` type [#614] -### Deprecated +### Removed - `Error::INTERNAL_START` and `Error::CUSTOM_START` associated constants [#614] [#570]: https://github.com/rust-random/getrandom/pull/570 diff --git a/src/error.rs b/src/error.rs index f9946147..4c66e8be 100644 --- a/src/error.rs +++ b/src/error.rs @@ -45,18 +45,10 @@ impl Error { /// Encountered an unexpected situation which should not happen in practice. pub const UNEXPECTED: Error = Self::new_internal(2); - #[deprecated] - #[doc(hidden)] - pub const INTERNAL_START: u32 = 1 << 31; - - #[deprecated] - #[doc(hidden)] - pub const CUSTOM_START: u32 = (1 << 31) + (1 << 30); - /// Internal errors can be in the range of 2^16..2^17 - const INTERNAL_START2: RawOsError = 1 << 16; + const INTERNAL_START: RawOsError = 1 << 16; /// Custom errors can be in the range of 2^17..(2^17 + 2^16) - const CUSTOM_START2: RawOsError = 1 << 17; + const CUSTOM_START: RawOsError = 1 << 17; /// Creates a new instance of an `Error` from a particular OS error code. /// @@ -128,14 +120,14 @@ impl Error { /// Creates a new instance of an `Error` from a particular custom error code. pub const fn new_custom(n: u16) -> Error { // SAFETY: code > 0 as CUSTOM_START > 0 and adding `n` won't overflow `RawOsError`. - let code = Error::CUSTOM_START2 + (n as RawOsError); + let code = Error::CUSTOM_START + (n as RawOsError); Error(unsafe { NonZeroRawOsError::new_unchecked(code) }) } /// Creates a new instance of an `Error` from a particular internal error code. pub(crate) const fn new_internal(n: u16) -> Error { // SAFETY: code > 0 as INTERNAL_START > 0 and adding `n` won't overflow `RawOsError`. - let code = Error::INTERNAL_START2 + (n as RawOsError); + let code = Error::INTERNAL_START + (n as RawOsError); Error(unsafe { NonZeroRawOsError::new_unchecked(code) }) } From c8469beb8bc4cb2c5f3f6c9d9af84ac8806fa6fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Wed, 5 Mar 2025 19:51:57 +0300 Subject: [PATCH 11/17] Tweak `Error` docs --- src/error.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/error.rs b/src/error.rs index 4c66e8be..3bc992bf 100644 --- a/src/error.rs +++ b/src/error.rs @@ -24,14 +24,12 @@ cfg_if::cfg_if!( /// if so, which error code the OS gave the application. If such an error is /// encountered, please consult with your system documentation. /// -/// Internally this type is a NonZeroU32, with certain values reserved for -/// certain purposes, see [`Error::INTERNAL_START`] and [`Error::CUSTOM_START`]. -/// /// *If this crate's `"std"` Cargo feature is enabled*, then: /// - [`getrandom::Error`][Error] implements /// [`std::error::Error`](https://doc.rust-lang.org/std/error/trait.Error.html) /// - [`std::io::Error`](https://doc.rust-lang.org/std/io/struct.Error.html) implements /// [`From`](https://doc.rust-lang.org/std/convert/trait.From.html). + // note: on non-UEFI targets OS errors are represented as negative integers, // while on UEFI targets OS errors have the highest bit set to 1. #[derive(Copy, Clone, Eq, PartialEq)] From ccc1258e6c7ecf231d2fbb8d29593086cea5d97f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Wed, 5 Mar 2025 20:26:03 +0300 Subject: [PATCH 12/17] Fix v0.3.2 link in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 666c028d..16fdb371 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -561,7 +561,7 @@ Publish initial implementation. ## [0.0.0] - 2019-01-19 Publish an empty template library. -[0.3.2]: https://github.com/rust-random/getrandom/compare/v0.3.0...v0.3.2 +[0.3.2]: https://github.com/rust-random/getrandom/compare/v0.3.1...v0.3.2 [0.3.1]: https://github.com/rust-random/getrandom/compare/v0.3.0...v0.3.1 [0.3.0]: https://github.com/rust-random/getrandom/compare/v0.2.15...v0.3.0 [0.2.15]: https://github.com/rust-random/getrandom/compare/v0.2.14...v0.2.15 From 4e8a6af4718ec5ef34fa4bf94009f128995573f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Fri, 7 Mar 2025 12:22:15 +0300 Subject: [PATCH 13/17] Rename `Error::from_os_error` to `from_neg_error_code`. Add `Error::from_uefi_code` --- src/backends/hermit.rs | 2 +- src/backends/linux_raw.rs | 2 +- src/backends/solid.rs | 2 +- src/backends/wasi_p1.rs | 2 +- src/error.rs | 35 ++++++++++++++++++++--------------- src/util_libc.rs | 2 +- 6 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/backends/hermit.rs b/src/backends/hermit.rs index 43bf5ff7..34d7cdbb 100644 --- a/src/backends/hermit.rs +++ b/src/backends/hermit.rs @@ -45,7 +45,7 @@ pub fn fill_inner(mut dest: &mut [MaybeUninit]) -> Result<(), Error> { } code => { let code = i32::try_from(code).map_err(|_| Error::UNEXPECTED)?; - return Err(Error::from_os_error(code)); + return Err(Error::from_neg_error_code(code)); } } } diff --git a/src/backends/linux_raw.rs b/src/backends/linux_raw.rs index 953a6584..4a59eef0 100644 --- a/src/backends/linux_raw.rs +++ b/src/backends/linux_raw.rs @@ -129,7 +129,7 @@ pub fn fill_inner(mut dest: &mut [MaybeUninit]) -> Result<(), Error> { Err(_) if ret == EINTR => continue, Err(_) => { let code = i32::try_from(ret).map_err(|_| Error::UNEXPECTED)?; - return Err(Error::from_os_error(code)); + return Err(Error::from_neg_error_code(code)); } } } diff --git a/src/backends/solid.rs b/src/backends/solid.rs index 467e3db2..caa773f8 100644 --- a/src/backends/solid.rs +++ b/src/backends/solid.rs @@ -14,6 +14,6 @@ pub fn fill_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { if ret >= 0 { Ok(()) } else { - Err(Error::from_os_error(ret)) + Err(Error::from_neg_error_code(ret)) } } diff --git a/src/backends/wasi_p1.rs b/src/backends/wasi_p1.rs index 7a29a979..76dbc6d0 100644 --- a/src/backends/wasi_p1.rs +++ b/src/backends/wasi_p1.rs @@ -21,6 +21,6 @@ pub fn fill_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { let ret = unsafe { random_get(dest.as_mut_ptr() as i32, dest.len() as i32) }; match ret { 0 => Ok(()), - code => Err(Error::from_os_error(code)), + code => Err(Error::from_neg_error_code(code)), } } diff --git a/src/error.rs b/src/error.rs index 3bc992bf..04d6b19c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -48,21 +48,25 @@ impl Error { /// Custom errors can be in the range of 2^17..(2^17 + 2^16) const CUSTOM_START: RawOsError = 1 << 17; - /// Creates a new instance of an `Error` from a particular OS error code. - /// - /// This method is analogous to [`std::io::Error::from_raw_os_error()`][1], - /// except that it works in `no_std` contexts and `code` will be - /// replaced with `Error::UNEXPECTED` in unexpected cases. - /// - /// [1]: https://doc.rust-lang.org/std/io/struct.Error.html#method.from_raw_os_error + /// Creates a new instance of an `Error` from a negative error code. #[allow(dead_code)] - pub(super) fn from_os_error(code: RawOsError) -> Self { - match NonZeroRawOsError::new(code) { - #[cfg(target_os = "uefi")] - Some(code) if code.get() & UEFI_ERROR_FLAG != 0 => Self(code), - #[cfg(not(target_os = "uefi"))] - Some(code) if code.get() < 0 => Self(code), - _ => Self::UNEXPECTED, + pub(super) fn from_neg_error_code(code: RawOsError) -> Self { + if code < 0 { + let code = NonZeroRawOsError::new(code).expect("`code` is negative"); + Self(code) + } else { + Error::UNEXPECTED + } + } + + /// Creates a new instance of an `Error` from an UEFI error code. + #[cfg(target_os = "uefi")] + pub(super) fn from_uefi_code(code: RawOsError) -> Self { + if code.get() & UEFI_ERROR_FLAG != 0 { + let code = NonZeroRawOsError::new(code).expect("The highest bit of `code` is set to 1"); + Self(code) + } else { + Self::UNEXPECTED } } @@ -82,7 +86,8 @@ impl Error { pub fn raw_os_error(self) -> Option { let code = self.0.get(); - // note: in this method we need to cover only backends which rely on `Error::from_os_error`, + // note: in this method we need to cover only backends which rely on + // `Error::{from_error_code, from_errno, from_uefi_code}` methods, // on all other backends this method always returns `None`. #[cfg(target_os = "uefi")] diff --git a/src/util_libc.rs b/src/util_libc.rs index 5ec07ee8..80003013 100644 --- a/src/util_libc.rs +++ b/src/util_libc.rs @@ -41,7 +41,7 @@ pub(crate) fn last_os_error() -> Error { let code = errno .checked_neg() .expect("Positive number can be always negated"); - Error::from_os_error(code) + Error::from_neg_error_code(code) } else { Error::ERRNO_NOT_POSITIVE } From 51d158b97d96d588062530e4c52397c1eded3367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Fri, 7 Mar 2025 12:24:04 +0300 Subject: [PATCH 14/17] Remove TEMP_EFI_ERROR --- src/backends/efi_rng.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/backends/efi_rng.rs b/src/backends/efi_rng.rs index 5c642a3b..768c8cc8 100644 --- a/src/backends/efi_rng.rs +++ b/src/backends/efi_rng.rs @@ -43,7 +43,7 @@ fn init() -> Result, Error> { }; if ret.is_error() { - return Err(Error::TEMP_EFI_ERROR); + return Err(Error::from_uefi_code(ret.as_usize())); } let handles_len = buf_size / HANDLE_SIZE; @@ -112,7 +112,7 @@ pub fn fill_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { }; if ret.is_error() { - Err(Error::TEMP_EFI_ERROR) + Err(Error::from_uefi_code(ret.as_usize())) } else { Ok(()) } @@ -121,5 +121,4 @@ pub fn fill_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { impl Error { pub(crate) const BOOT_SERVICES_UNAVAILABLE: Error = Self::new_internal(10); pub(crate) const NO_RNG_HANDLE: Error = Self::new_internal(11); - pub(crate) const TEMP_EFI_ERROR: Error = Self::new_internal(12); } From 839587056b536f921f4d18d23580c0e002e504a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Fri, 7 Mar 2025 12:26:02 +0300 Subject: [PATCH 15/17] Fix uefi --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 04d6b19c..f3319fc6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -62,7 +62,7 @@ impl Error { /// Creates a new instance of an `Error` from an UEFI error code. #[cfg(target_os = "uefi")] pub(super) fn from_uefi_code(code: RawOsError) -> Self { - if code.get() & UEFI_ERROR_FLAG != 0 { + if code & UEFI_ERROR_FLAG != 0 { let code = NonZeroRawOsError::new(code).expect("The highest bit of `code` is set to 1"); Self(code) } else { From 53980214204f25f8a162359df60eb6db0be2bad6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Fri, 7 Mar 2025 12:30:56 +0300 Subject: [PATCH 16/17] Gate `from_neg_error_code` on non-UEFI targets --- src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/error.rs b/src/error.rs index f3319fc6..cbdc69be 100644 --- a/src/error.rs +++ b/src/error.rs @@ -49,6 +49,7 @@ impl Error { const CUSTOM_START: RawOsError = 1 << 17; /// Creates a new instance of an `Error` from a negative error code. + #[cfg(not(target_os = "uefi"))] #[allow(dead_code)] pub(super) fn from_neg_error_code(code: RawOsError) -> Self { if code < 0 { From 897ae4a91b1b246979b339ba7c098f6fa41dad9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Fri, 7 Mar 2025 12:32:45 +0300 Subject: [PATCH 17/17] Add `#[allow(dead_code)]` for `from_uefi_code` --- src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/error.rs b/src/error.rs index cbdc69be..13f3121f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -62,6 +62,7 @@ impl Error { /// Creates a new instance of an `Error` from an UEFI error code. #[cfg(target_os = "uefi")] + #[allow(dead_code)] pub(super) fn from_uefi_code(code: RawOsError) -> Self { if code & UEFI_ERROR_FLAG != 0 { let code = NonZeroRawOsError::new(code).expect("The highest bit of `code` is set to 1");