Merge pull request #529 from nicholasbishop/bishop-revision-formatting

Fixes and improvements for `Revision`

Author: phip1611

CHANGELOG.md

@@ -14,6 +14,14 @@
 - The conversion methods on `DevicePathToText` and `DevicePathFromText`
   now return a `uefi::Result` instead of an `Option`.
 - `Event` is now a newtype around `NonNull<c_void>` instead of `*mut c_void`.
+- Changed `SystemTable::firmware_revision` to return a `u32` instead of
+  a `Revision`. The firmware revision's format is vendor specific and
+  may not have the same semantics as the UEFI revision.
+- Changed `Revision` to `repr(transparent)`.
+- Add `Revision::EFI_2_100` constant.
+- The `Revision` type now implements `Display` with correct formatting
+  for all UEFI versions. The custom `Debug` impl has been removed and
+  replaced with a derived `Debug` impl.
 
 ## uefi-macros - [Unreleased]
 

src/table/revision.rs

@@ -6,7 +6,32 @@ use core::fmt;
 ///
 /// The minor revision number is incremented on minor changes,
 /// it is stored as a two-digit binary-coded decimal.
-#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
+///
+/// # Display format
+///
+/// For major revision 2 and later, if the lower minor digit is zero,
+/// the revision is formatted as "major.minor-upper". Otherwise it's
+/// formatted as "major.minor-upper.minor-lower". This format is
+/// described in the "EFI System Table" section of the UEFI
+/// Specification.
+///
+/// Prior to major version 2, the revision is always formatted as
+/// "major.minor", with minor left-padded with zero if minor-upper is
+/// zero.
+///
+/// Examples:
+///
+/// ```
+/// use uefi::table::Revision;
+/// assert_eq!(Revision::EFI_1_02.to_string(), "1.02");
+/// assert_eq!(Revision::EFI_1_10.to_string(), "1.10");
+/// assert_eq!(Revision::EFI_2_00.to_string(), "2.0");
+/// assert_eq!(Revision::EFI_2_30.to_string(), "2.3");
+/// assert_eq!(Revision::EFI_2_31.to_string(), "2.3.1");
+/// assert_eq!(Revision::EFI_2_100.to_string(), "2.10");
+/// ```
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
+#[repr(transparent)]
 pub struct Revision(u32);
 
 // Allow missing docs, there's nothing useful to document about these
@@ -26,6 +51,7 @@ impl Revision {
     pub const EFI_2_70: Self = Self::new(2, 70);
     pub const EFI_2_80: Self = Self::new(2, 80);
     pub const EFI_2_90: Self = Self::new(2, 90);
+    pub const EFI_2_100: Self = Self::new(2, 100);
 }
 
 impl Revision {
@@ -48,13 +74,20 @@ impl Revision {
     }
 }
 
-impl fmt::Debug for Revision {
-    /// Formats the revision in the `major.minor.patch` format.
+impl fmt::Display for Revision {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let (major, minor) = (self.major(), self.minor());
-        let minor = minor / 10;
-        let patch = minor % 10;
-        write!(f, "{major}.{minor}.{patch}")
+
+        if major < 2 {
+            write!(f, "{major}.{minor:02}")
+        } else {
+            let (minor, patch) = (minor / 10, minor % 10);
+            if patch == 0 {
+                write!(f, "{major}.{minor}")
+            } else {
+                write!(f, "{major}.{minor}.{patch}")
+            }
+        }
     }
 }
 

src/table/system.rs

@@ -58,7 +58,7 @@ impl<View: SystemTableView> SystemTable<View> {
     }
 
     /// Return the firmware revision
-    pub fn firmware_revision(&self) -> Revision {
+    pub fn firmware_revision(&self) -> u32 {
         self.table.fw_revision
     }
 
@@ -296,8 +296,7 @@ struct SystemTableImpl {
     header: Header,
     /// Null-terminated string representing the firmware's vendor.
     fw_vendor: *const Char16,
-    /// Revision of the UEFI specification the firmware conforms to.
-    fw_revision: Revision,
+    fw_revision: u32,
     stdin_handle: Handle,
     stdin: *mut text::Input,
     stdout_handle: Handle,