Skip to content

Conversation

@Ayush1325
Copy link
Contributor

At least on OVMF, some files copied over from linux file system seem to have invalid time (year = 1980 and everything else 0). Since Rust allows time to be optional, and we can return error, that seems to be the way to go for now.

@rustbot label +O-UEFI

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 2, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 2, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@Ayush1325
Copy link
Contributor Author

@joboet Can you take a look at this since it kinda follows https://github.com/rust-lang/rust/pull/13891833

Copy link
Member

@joboet joboet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error messages should start in lowercase, apart from that this looks fine.

View changes since this review

@Ayush1325
Copy link
Contributor Author

The error messages should start in lowercase, apart from that this looks fine.

View changes since this review

Fixed

pub(crate) fn uefi_to_systemtime(mut time: r_efi::efi::Time) -> SystemTime {
pub(crate) fn uefi_to_systemtime(mut time: r_efi::efi::Time) -> Option<SystemTime> {
time.timezone = if time.timezone == r_efi::efi::UNSPECIFIED_TIMEZONE {
time::system_time_internal::now().unwrap().timezone
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a panic message here (and in systemtime_to_uefi) as well, like in SystemTime::now. Or maybe make system_time_internal::now() panic directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since all users of system_time_internal::now() were calling unwrap on it, I have made the function itself panic.

At least on OVMF, some files copied over from linux file system seem
to have invalid time (year = 1980 and everything else 0). Since Rust
allows time to be optional and we can return error, that seems to be
the way to go for now.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
@joboet
Copy link
Member

joboet commented Dec 2, 2025

Great, r=me once CI passes.
@bors delegate+

@bors
Copy link
Collaborator

bors commented Dec 2, 2025

✌️ @Ayush1325, you can now approve this pull request!

If @joboet told you to "r=me" after making some further change, please make that change, then do @bors r=@joboet

@joboet joboet assigned joboet and unassigned Mark-Simulacrum Dec 2, 2025
@Ayush1325
Copy link
Contributor Author

@bors r=@joboet

@bors
Copy link
Collaborator

bors commented Dec 2, 2025

📌 Commit 1331c35 has been approved by joboet

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 2, 2025
bors added a commit that referenced this pull request Dec 2, 2025
Rollup of 5 pull requests

Successful merges:

 - #146436 (Slice iter cleanup)
 - #148250 (array_chunks: slightly improve docs)
 - #148678 (Merge E0412 into E0425)
 - #149520 (also introduce Peekable::next_if_map_mut next to next_if_map)
 - #149538 (std: sys: fs: uefi: Make time in FileAttr optional)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3b99d6b into rust-lang:main Dec 3, 2025
11 checks passed
rust-timer added a commit that referenced this pull request Dec 3, 2025
Rollup merge of #149538 - Ayush1325:uefi-fs-time-fix, r=joboet

std: sys: fs: uefi: Make time in FileAttr optional

At least on OVMF, some files copied over from linux file system seem to have invalid time (year = 1980 and everything else 0). Since Rust allows time to be optional, and we can return error, that seems to be the way to go for now.

`@rustbot` label +O-UEFI
@rustbot rustbot added this to the 1.93.0 milestone Dec 3, 2025
@Ayush1325 Ayush1325 deleted the uefi-fs-time-fix branch December 3, 2025 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-UEFI UEFI S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants