uefi-raw: Make TimeError more descriptive

When returning a TimeError, it'd be helpful to specify to the user which field
is invalid so that it can be handled accordingly, or at least communicated.
This change does this by reimplementing TimeError as an enum instead of a
struct, allowing for specification and more verbose error display.

Author: andre-braga

uefi-raw/CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Changed
 - `maximum_capsule_size` of `query_capsule_capabilities` now takes a *mut u64 instead of a *mut usize.
 - `ResetType` now derives the `Default` trait.
+- `TimeError` is now implemented in uefi-raw as an enum instead of a struct in uefi. One can now specify what field of `Time` is invalid, and `Time::is_valid` has been updated accordingly. 
 
 # uefi-raw - 0.5.2 (2024-04-19)
 

uefi-raw/src/time.rs

@@ -3,6 +3,42 @@
 use bitflags::bitflags;
 use core::fmt::{self, Display, Formatter};
 
+/// Error returned by [`Time`] methods if the specified field
+/// is outside of the valid range.
+#[derive(Debug)]
+pub enum TimeError {
+    Year,
+    Month,
+    Day,
+    Hour,
+    Minute,
+    Second,
+    Nanosecond,
+    TimeZone,
+    Daylight,
+    ByteConversion,
+}
+
+impl Display for TimeError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Year => write!(f, "year not within `1900..=9999`"),
+            Self::Month => write!(f, "month not within `1..=12"),
+            Self::Day => write!(f, "day not within `1..=31`"),
+            Self::Hour => write!(f, "hour not within `0..=23`"),
+            Self::Minute => write!(f, "minute not within `0..=59`"),
+            Self::Second => write!(f, "second not within `0..=59`"),
+            Self::Nanosecond => write!(f, "nanosecond not within `0..=999_999_999`"),
+            Self::TimeZone => write!(
+                f,
+                "time_zone not `Time::UNSPECIFIED_TIMEZONE` nor within `-1440..=1440`"
+            ),
+            Self::Daylight => write!(f, "unknown bits set for daylight"),
+            Self::ByteConversion => write!(f, "byte splice not large enough to fit `Time`"),
+        }
+    }
+}
+
 /// Date and time representation.
 #[derive(Debug, Default, Copy, Clone, Eq)]
 #[repr(C)]
@@ -64,18 +100,29 @@ impl Time {
         }
     }
 
-    /// True if all fields are within valid ranges, false otherwise.
-    #[must_use]
-    pub fn is_valid(&self) -> bool {
-        (1900..=9999).contains(&self.year)
-            && (1..=12).contains(&self.month)
-            && (1..=31).contains(&self.day)
-            && self.hour <= 23
-            && self.minute <= 59
-            && self.second <= 59
-            && self.nanosecond <= 999_999_999
-            && ((-1440..=1440).contains(&self.time_zone)
-                || self.time_zone == Self::UNSPECIFIED_TIMEZONE)
+    /// `Ok()` if all fields are within valid ranges, `Err(TimeError)` otherwise.
+    pub fn is_valid(&self) -> Result<(), TimeError> {
+        if !(1900..=9999).contains(&self.year) {
+            Err(TimeError::Year)
+        } else if !(1..=12).contains(&self.month) {
+            Err(TimeError::Month)
+        } else if !(1..=31).contains(&self.day) {
+            Err(TimeError::Day)
+        } else if self.hour > 23 {
+            Err(TimeError::Hour)
+        } else if self.minute > 59 {
+            Err(TimeError::Minute)
+        } else if self.second > 59 {
+            Err(TimeError::Second)
+        } else if self.nanosecond > 999_999_999 {
+            Err(TimeError::Nanosecond)
+        } else if !((-1440..=1440).contains(&self.time_zone)
+            || self.time_zone == Self::UNSPECIFIED_TIMEZONE)
+        {
+            Err(TimeError::TimeZone)
+        } else {
+            Ok(())
+        }
     }
 }
 

uefi/CHANGELOG.md

@@ -14,6 +14,7 @@
 - Added `TryFrom<&[u8]>` for `DevicePathHeader`, `DevicePathNode` and `DevicePath`.
 - Added `ByteConversionError`.
 - Re-exported `CapsuleFlags`.
+- `TimeError` is now implemented in uefi-raw as an enum instead of a struct in uefi. One can now specify what field of `Time` is invalid, and `Time::is_valid` has been updated accordingly. 
 
 ## Changed
 - `SystemTable::exit_boot_services` is now `unsafe`. See that method's

uefi/src/table/runtime.rs

@@ -11,7 +11,7 @@ pub use uefi_raw::capsule::{CapsuleBlockDescriptor, CapsuleFlags, CapsuleHeader}
 pub use uefi_raw::table::runtime::{
     ResetType, TimeCapabilities, VariableAttributes, VariableVendor,
 };
-pub use uefi_raw::time::Daylight;
+pub use uefi_raw::time::{Daylight, TimeError};
 pub use uefi_raw::PhysicalAddress;
 
 #[cfg(feature = "alloc")]
@@ -371,19 +371,6 @@ pub struct TimeParams {
     pub daylight: Daylight,
 }
 
-/// Error returned by [`Time`] methods if the input is outside the valid range.
-#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
-pub struct TimeError;
-
-impl Display for TimeError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{self:?}")
-    }
-}
-
-#[cfg(feature = "unstable")]
-impl core::error::Error for TimeError {}
-
 impl Time {
     /// Unspecified Timezone/local time.
     const UNSPECIFIED_TIMEZONE: i16 = uefi_raw::time::Time::UNSPECIFIED_TIMEZONE;
@@ -404,11 +391,8 @@ impl Time {
             daylight: params.daylight,
             pad2: 0,
         });
-        if time.is_valid() {
-            Ok(time)
-        } else {
-            Err(TimeError)
-        }
+
+        time.is_valid().map(|_| time)
     }
 
     /// Create an invalid `Time` with all fields set to zero. This can
@@ -422,9 +406,8 @@ impl Time {
         Self(uefi_raw::time::Time::invalid())
     }
 
-    /// True if all fields are within valid ranges, false otherwise.
-    #[must_use]
-    pub fn is_valid(&self) -> bool {
+    /// `Ok()` if all fields are within valid ranges, `Err(TimeError)` otherwise.
+    pub fn is_valid(&self) -> core::result::Result<(), TimeError> {
         self.0.is_valid()
     }