uefi: 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/CHANGELOG.md

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

uefi/src/table/runtime.rs

@@ -371,19 +371,41 @@ 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;
+/// Error returned by [`Time`] methods if the specified field
+/// is outside of the valid range.
+#[allow(missing_docs)]
+#[derive(Debug)]
+pub enum TimeError {
+    Year,
+    Month,
+    Day,
+    Hour,
+    Minute,
+    Second,
+    Nanosecond,
+    Timezone,
+    Daylight,
+}
 
 impl Display for TimeError {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{self:?}")
+        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"),
+        }
     }
 }
 
-#[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 +426,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,10 +441,29 @@ 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 {
-        self.0.is_valid()
+    /// `Ok()` if all fields are within valid ranges, `Err(TimeError)` otherwise.
+    pub fn is_valid(&self) -> core::result::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 self.time_zone().is_some()
+            && !((-1440..=1440).contains(&self.time_zone().unwrap()))
+        {
+            Err(TimeError::Timezone)
+        } else {
+            Ok(())
+        }
     }
 
     /// Query the year.