uefi: Add integration with crates jiff and time for struct Time#1911
uefi: Add integration with crates jiff and time for struct Time#1911
jiff and time for struct Time#1911Conversation
phip1611
force-pushed
the
uefi-raw-time-v3
branch
from
d5d571f to
b55aa67
Compare
|
@nicholasbishop thanks for your valuable review and feedback in #1896 and #1899. I think we are now the closest to have something useful and mergable. Looking forward to your feedback! |
phip1611
force-pushed
the
uefi-raw-time-v3
branch
from
b55aa67 to
80ffbb1
Compare
This is a preparation to integrate the popular time-related crates `jiff` and `time` with `struct Time` of the `uefi` crate.
I split the time-related type definitions from the time-related runtime UEFI services. This is a preparation to integrate the popular time-related crates `jiff` and `time` with `struct Time` of the `uefi` crate.
phip1611
force-pushed
the
uefi-raw-time-v3
branch
from
80ffbb1 to
f9b11f2
Compare
This module provides the foundational plumbing for future time-related crate integrations. It introduces common error types, including a unified opaque ConversionError to standardize conversion failures across crates. While integration with crates like time and jiff is planned in subsequent commits, this module lays the groundwork, ensuring a consistent and minimal API for handling errors once those integrations are implemented.
This allows to pass `jiff::{DateTime, Zoned}` and
`time::{PrimitiveDateTime,OffsetDateTime}`.
phip1611
force-pushed
the
uefi-raw-time-v3
branch
from
f9b11f2 to
0534af3
Compare
nicholasbishop
left a comment
nicholasbishop
left a comment
There was a problem hiding this comment.
Generally looks good to me, but I haven't reviewed in detail yet. Let's break it apart some. I think start with a new PR to move the Time stuff to a new module like uefi::runtime::time, then a PR to add time, then jiff.
| /// Undefined behavior could happen if multiple tasks try to | ||
| /// use this function at the same time without synchronisation. | ||
| pub unsafe fn set_time(time: &Time) -> Result { | ||
| pub unsafe fn set_time<T: Clone + TryInto<Time>>(time: &T) -> Result { |
There was a problem hiding this comment.
I'd like to revert this change. Time conversion is fraught enough that keeping it explicit seems worthwhile to me, especially since set_time is unlikely to be called frequently in code so implicit conversion probably isn't doing much to make calling code more readable.
| //! functions after exiting boot services; see the "Calling Convention" section | ||
| //! of the UEFI specification for details. | ||
|
|
||
| mod time_defs; |
There was a problem hiding this comment.
nit: module could be called just time, I don't think defs is adding any clarity.
| Ok(()) | ||
| } | ||
|
|
||
| /// Date and time representation. |
There was a problem hiding this comment.
Let's do this move of Time stuff to a new module in its own PR and merge that first.
2 participants
This is my third design, superseding the outdated approaches #1896 and #1899.
Motivation
My high-level goal is to make
struct Timemore useable, e.g., compare dates (before, after, ...).Design
This PR integrates
struct Timeof theueficrate with https://crates.io/crates/time and https://crates.io/crates/jiff. I decided for the following design:TryFromand no specific constructorsError
There is a single opaque
ConversionErrorshared by allTryFromimpls.Implementation
timecratestruct Time <--> PrimitiveDateTime(without timezone)struct Time <--> OffsetDateTime(with timezone)jiffcratestruct Time <--> DateTime(without timezone)struct Time <--> Zoned(with timezone)