Improve Time struct #395
Merged
Improve Time struct #395
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Add a `TimeParams` struct with public fields corresponding to all the non-padding fields of `Time`, and change `Time::new` to take that as input. That improves callsites of `Time::new` so that you can see what the numeric values correspond to. See `known_disk.rs` for an example. * Add `Time::invalid` for use with `File::set_info`. When setting file info, a fully-zero-time indicates the attribute should not be updated. I think ideally we'd have `Time` always represent a valid value, and use `Option<Time>::None` to represent an invalid time similar to what we do with `Handle`s, but I don't think Rust currently allows us to indicate that all-zero-Time can be used for the Option::None niche. * Add `Time::is_invalid` to check that all fields are in the valid range. * Update Time's docstring to be more general. Although `EFI_TIME` in the spec says it's for the current time, it's used elsewhere too (like `EFI_FILE_INFO`). * Fix the docstring for `Daylight`, which was accidentally copied from `MemoryAttribute`.
nicholasbishop
force-pushed
the
bishop-time-cleanup
branch
from
e48f118 to
15dcd78
Compare
|
LGTM! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a
TimeParamsstruct with public fields corresponding to all thenon-padding fields of
Time, and changeTime::newto take that asinput. That improves callsites of
Time::newso that you can see whatthe numeric values correspond to. See
known_disk.rsfor an example.Add
Time::invalidfor use withFile::set_info. When setting fileinfo, a fully-zero-time indicates the attribute should not be
updated. I think ideally we'd have
Timealways represent a validvalue, and use
Option<Time>::Noneto represent an invalid timesimilar to what we do with
Handles, but I don't think Rust currentlyallows us to indicate that all-zero-Time can be used for the
Option::None niche.
Add
Time::is_invalidto check that all fields are in the valid range.Update Time's docstring to be more general. Although
EFI_TIMEin thespec says it's for the current time, it's used elsewhere too (like
EFI_FILE_INFO).Fix the docstring for
Daylight, which was accidentally copied fromMemoryAttribute.