Tweak Error representation

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
 

src/backends/hermit.rs

@@ -44,10 +44,8 @@ pub fn fill_inner(mut dest: &mut [MaybeUninit<u8>]) -> 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));
             }
         }
     }

src/backends/linux_raw.rs

@@ -128,10 +128,7 @@ pub fn fill_inner(mut dest: &mut [MaybeUninit<u8>]) -> 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));
             }
         }

src/backends/solid.rs

@@ -14,8 +14,6 @@ pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> 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))
     }
 }

src/backends/wasi_p1.rs

@@ -21,11 +21,6 @@ pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> 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)),
     }
 }

src/error.rs

@@ -1,15 +1,20 @@
 #[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")] {
+        // 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);
     } else {
         type RawOsError = i32;
+        type NonZeroRawOsError = core::num::NonZeroI32;
     }
 );
 
@@ -27,8 +32,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<getrandom::Error>`](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 +45,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]
+    #[doc(hidden)]
     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]
+    #[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;
+    /// 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 +91,38 @@ impl Error {
     #[inline]
     pub fn raw_os_error(self) -> Option<RawOsError> {
         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> {
@@ -176,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::<Error>(), 4);
-        assert_eq!(size_of::<Result<(), Error>>(), 4);
-    }
-}

src/util_libc.rs

@@ -34,14 +34,16 @@ cfg_if! {
 }
 
 pub(crate) fn last_os_error() -> Error {
-    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() };
 
-    // c_int-to-u32 conversion is lossless for nonnegative values if they are the same size.
-    const _: () = assert!(core::mem::size_of::<libc::c_int>() == core::mem::size_of::<u32>());
-
-    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
     }
 }