Skip to content

Fixes and improvements for Revision #529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 26, 2022

Conversation

nicholasbishop
Copy link
Member

  • Changed firmware-revision to an opaque u32 instead of a Revision. The Revision type is specific to a UEFI revision, but the firmware revision doesn't have any defined semantics (it's not necessarily a major.minor format).
  • Fixed the repr for Revision to be transparent
  • Added EFI_2_100 for the latest spec revision
  • Added a new Display impl for Revision that formats correctly for all spec versions.
  • Replaced the custom Debug impl with a derived impl; the new Display impl can be used for pretty formatting.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits): See the
    Rewriting History guide for
    help.
  • Update the changelog (if necessary)

@@ -12,6 +12,9 @@
`Address` always take a 64-bit value, regardless of target platform.
- The conversion methods on `DevicePathToText` and `DevicePathFromText`
now return a `uefi::Result` instead of an `Option`.
- Changed `SystemTable::firmware_revision` to return a `u32` instead of
Copy link
Member

Choose a reason for hiding this comment

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

good change, good catch!

@phip1611
Copy link
Member

Very nice! Will approve once you rebased this and all threads are resolved

@nicholasbishop nicholasbishop force-pushed the bishop-revision-formatting branch from e9932f1 to 57b45a7 Compare October 12, 2022 14:41
@phip1611
Copy link
Member

phip1611 commented Oct 24, 2022

It seems like you have to solve a conflict in the CHANGELOG.md file

@nicholasbishop nicholasbishop force-pushed the bishop-revision-formatting branch from 57b45a7 to dc5d019 Compare October 24, 2022 14:54
Prior to this commit, `SystemTable::firmware_revision` returned a
`Revision`. But the `Revision` type is specific to a UEFI spec
revision. The format of the vendor revision is vendor specific, so
change it to an opaque `u32` since we don't know what its semantics are.
Add a `Display` impl that correctly formats the revision for all spec
versions. There are docstring tests that demonstrate it in action.

Also removed the `Debug` impl and replaced with a derived impl.
@nicholasbishop nicholasbishop force-pushed the bishop-revision-formatting branch from dc5d019 to ad01c57 Compare October 26, 2022 12:51
@nicholasbishop
Copy link
Member Author

Rebased to fix conflicts with the clippy lint fix.

@phip1611 phip1611 merged commit 80065b5 into rust-osdev:main Oct 26, 2022
@nicholasbishop nicholasbishop deleted the bishop-revision-formatting branch October 26, 2022 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants