Skip to content

Commit 1cd4aa9

Browse files
committed
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 adding bool fields to the TimeError struct, representing the validity of each field of a Time struct.
1 parent bd7caae commit 1cd4aa9

File tree

2 files changed

+90
-16
lines changed

2 files changed

+90
-16
lines changed

uefi/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
- Added `TryFrom<&[u8]>` for `DevicePathHeader`, `DevicePathNode` and `DevicePath`.
1515
- Added `ByteConversionError`.
1616
- Re-exported `CapsuleFlags`.
17+
- One can now specify in `TimeError` what fields of `Time` are outside its valid range. `Time::is_valid` has been updated accordingly.
1718

1819
## Changed
1920
- `SystemTable::exit_boot_services` is now `unsafe`. See that method's

uefi/src/table/runtime.rs

Lines changed: 89 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -371,19 +371,66 @@ pub struct TimeParams {
371371
pub daylight: Daylight,
372372
}
373373

374-
/// Error returned by [`Time`] methods if the input is outside the valid range.
375-
#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
376-
pub struct TimeError;
374+
/// Error returned by [`Time`] methods. A bool value of `true` means
375+
/// the specified field is outside of its valid range.
376+
#[allow(missing_docs)]
377+
#[derive(Debug, Default, PartialEq, Eq)]
378+
pub struct TimeError {
379+
pub year: bool,
380+
pub month: bool,
381+
pub day: bool,
382+
pub hour: bool,
383+
pub minute: bool,
384+
pub second: bool,
385+
pub nanosecond: bool,
386+
pub timezone: bool,
387+
pub daylight: bool,
388+
}
389+
390+
#[cfg(feature = "unstable")]
391+
impl core::error::Error for TimeError {}
377392

378393
impl Display for TimeError {
379394
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
380-
write!(f, "{self:?}")
395+
if *self == TimeError::default() {
396+
write!(f, "No erroneous fields found in EFI Time.")?;
397+
} else {
398+
writeln!(f, "Erroneous fields found in EFI Time:")?;
399+
if self.year {
400+
writeln!(f, "year not within `1900..=9999`")?;
401+
}
402+
if self.month {
403+
writeln!(f, "month not within `1..=12")?;
404+
}
405+
if self.day {
406+
writeln!(f, "day not within `1..=31`")?;
407+
}
408+
if self.hour {
409+
writeln!(f, "hour not within `0..=23`")?;
410+
}
411+
if self.minute {
412+
writeln!(f, "minute not within `0..=59`")?;
413+
}
414+
if self.second {
415+
writeln!(f, "second not within `0..=59`")?;
416+
}
417+
if self.nanosecond {
418+
writeln!(f, "nanosecond not within `0..=999_999_999`")?;
419+
}
420+
if self.timezone {
421+
writeln!(
422+
f,
423+
"time_zone not `Time::UNSPECIFIED_TIMEZONE` nor within `-1440..=1440`"
424+
)?;
425+
}
426+
if self.daylight {
427+
writeln!(f, "unknown bits set for daylight")?;
428+
}
429+
}
430+
Ok(())
381431
}
382432
}
383433

384-
#[cfg(feature = "unstable")]
385-
impl core::error::Error for TimeError {}
386-
387434
impl Time {
388435
/// Unspecified Timezone/local time.
389436
const UNSPECIFIED_TIMEZONE: i16 = uefi_raw::time::Time::UNSPECIFIED_TIMEZONE;
@@ -404,11 +451,8 @@ impl Time {
404451
daylight: params.daylight,
405452
pad2: 0,
406453
});
407-
if time.is_valid() {
408-
Ok(time)
409-
} else {
410-
Err(TimeError)
411-
}
454+
455+
time.is_valid().map(|_| time)
412456
}
413457

414458
/// Create an invalid `Time` with all fields set to zero. This can
@@ -422,10 +466,39 @@ impl Time {
422466
Self(uefi_raw::time::Time::invalid())
423467
}
424468

425-
/// True if all fields are within valid ranges, false otherwise.
426-
#[must_use]
427-
pub fn is_valid(&self) -> bool {
428-
self.0.is_valid()
469+
/// `Ok()` if all fields are within valid ranges, `Err(TimeError)` otherwise.
470+
pub fn is_valid(&self) -> core::result::Result<(), TimeError> {
471+
let mut err = TimeError::default();
472+
if !(1900..=9999).contains(&self.year()) {
473+
err.year = true;
474+
}
475+
if !(1..=12).contains(&self.month()) {
476+
err.month = true;
477+
}
478+
if !(1..=31).contains(&self.day()) {
479+
err.day = true;
480+
}
481+
if self.hour() > 23 {
482+
err.hour = true;
483+
}
484+
if self.minute() > 59 {
485+
err.minute = true;
486+
}
487+
if self.second() > 59 {
488+
err.second = true;
489+
}
490+
if self.nanosecond() > 999_999_999 {
491+
err.nanosecond = true;
492+
}
493+
if self.time_zone().is_some() && !((-1440..=1440).contains(&self.time_zone().unwrap())) {
494+
err.timezone = true;
495+
}
496+
// All fields are false, e.g., within their valid range.
497+
if err == TimeError::default() {
498+
Ok(())
499+
} else {
500+
Err(err)
501+
}
429502
}
430503

431504
/// Query the year.

0 commit comments

Comments
 (0)