Enhance Guid::from_values and Guid::fmt (#280)

Author: timrobertsdev

src/data_types/guid.rs

@@ -26,32 +26,31 @@ pub struct Guid {
 
 impl Guid {
     /// Creates a new GUID from its canonical representation
-    //
-    // FIXME: An unwieldy array of bytes must be used for the node ID until one
-    //        can assert that an u64 has its high 16-bits cleared in a const fn.
-    //        Once that is done, we can take an u64 to be even closer to the
-    //        canonical UUID/GUID format.
-    //
     pub const fn from_values(
         time_low: u32,
         time_mid: u16,
         time_high_and_version: u16,
         clock_seq_and_variant: u16,
-        node: [u8; 6],
+        node: u64,
     ) -> Self {
+        assert!(node.leading_zeros() >= 16, "node must be a 48-bit integer");
+        // intentional shadowing
+        let node = node.to_be_bytes();
+
         Guid {
             a: time_low,
             b: time_mid,
             c: time_high_and_version,
             d: [
                 (clock_seq_and_variant / 0x100) as u8,
                 (clock_seq_and_variant % 0x100) as u8,
-                node[0],
-                node[1],
+                // first two elements of node are ignored, we only want the low 48 bits
                 node[2],
                 node[3],
                 node[4],
                 node[5],
+                node[6],
+                node[7],
             ],
         }
     }
@@ -60,18 +59,17 @@ impl Guid {
 impl fmt::Display for Guid {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         let d = {
-            let (low, high) = (u16::from(self.d[0]), u16::from(self.d[1]));
-
-            (low << 8) | high
+            let mut buf = [0u8; 2];
+            buf[..].copy_from_slice(&self.d[0..2]);
+            u16::from_be_bytes(buf)
         };
 
-        // Extract and reverse byte order.
-        let e = self.d[2..8].iter().enumerate().fold(0, |acc, (i, &elem)| {
-            acc | {
-                let shift = (5 - i) * 8;
-                u64::from(elem) << shift
-            }
-        });
+        let e = {
+            let mut buf = [0u8; 8];
+            // first two elements of node are ignored, we only want the low 48 bits
+            buf[2..].copy_from_slice(&self.d[2..8]);
+            u64::from_be_bytes(buf)
+        };
 
         write!(
             fmt,
@@ -107,3 +105,32 @@ pub unsafe trait Identify {
 }
 
 pub use uefi_macros::unsafe_guid;
+
+#[cfg(test)]
+mod tests {
+    use uefi::unsafe_guid;
+    extern crate alloc;
+    use super::*;
+
+    #[test]
+    fn test_guid_display() {
+        assert_eq!(
+            alloc::format!(
+                "{}",
+                Guid::from_values(0x12345678, 0x9abc, 0xdef0, 0x1234, 0x56789abcdef0)
+            ),
+            "12345678-9abc-def0-1234-56789abcdef0"
+        );
+    }
+
+    #[test]
+    fn test_unsafe_guid() {
+        #[unsafe_guid("12345678-9abc-def0-1234-56789abcdef0")]
+        struct X;
+
+        assert_eq!(
+            X::GUID,
+            Guid::from_values(0x12345678, 0x9abc, 0xdef0, 0x1234, 0x56789abcdef0)
+        );
+    }
+}

src/proto/media/partition.rs

@@ -59,7 +59,7 @@ newtype_enum! {
             0x0000,
             0x0000,
             0x0000,
-            [0x00, 0x00, 0x00, 0x00, 0x00, 0x00],
+            0x000000000000,
         ),
 
         /// EFI System Partition.
@@ -68,7 +68,7 @@ newtype_enum! {
             0xf81f,
             0x11d2,
             0xba4b,
-            [0x00, 0xa0, 0xc9, 0x3e, 0xc9, 0x3b],
+            0x00a0c93ec93b,
         ),
 
         /// Partition containing a legacy MBR.
@@ -77,7 +77,7 @@ newtype_enum! {
             0x33e7,
             0x11d3,
             0x9d69,
-            [0x00, 0x08, 0xc7, 0x81, 0xf3, 0x9f],
+            0x0008c781f39f,
         ),
     }
 }

src/table/cfg.rs

@@ -26,54 +26,25 @@ pub struct ConfigTableEntry {
     /// Whether this is a physical or virtual address depends on the table.
     pub address: *const c_void,
 }
-
 /// Entry pointing to the old ACPI 1 RSDP.
-pub const ACPI_GUID: Guid = Guid::from_values(
-    0xeb9d2d30,
-    0x2d88,
-    0x11d3,
-    0x9a16,
-    [0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d],
-);
+pub const ACPI_GUID: Guid = Guid::from_values(0xeb9d2d30, 0x2d88, 0x11d3, 0x9a16, 0x0090273fc14d);
 
 ///Entry pointing to the ACPI 2 RSDP.
-pub const ACPI2_GUID: Guid = Guid::from_values(
-    0x8868e871,
-    0xe4f1,
-    0x11d3,
-    0xbc22,
-    [0x00, 0x80, 0xc7, 0x3c, 0x88, 0x81],
-);
+pub const ACPI2_GUID: Guid = Guid::from_values(0x8868e871, 0xe4f1, 0x11d3, 0xbc22, 0x0080c73c8881);
 
 /// Entry pointing to the SMBIOS 1.0 table.
-pub const SMBIOS_GUID: Guid = Guid::from_values(
-    0xeb9d2d31,
-    0x2d88,
-    0x11d3,
-    0x9a16,
-    [0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d],
-);
+pub const SMBIOS_GUID: Guid = Guid::from_values(0xeb9d2d31, 0x2d88, 0x11d3, 0x9a16, 0x0090273fc14d);
 
 /// Entry pointing to the SMBIOS 3.0 table.
-pub const SMBIOS3_GUID: Guid = Guid::from_values(
-    0xf2fd1544,
-    0x9794,
-    0x4a2c,
-    0x992e,
-    [0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94],
-);
+pub const SMBIOS3_GUID: Guid =
+    Guid::from_values(0xf2fd1544, 0x9794, 0x4a2c, 0x992e, 0xe5bbcf20e394);
 
 /// GUID of the UEFI properties table.
 ///
 /// The properties table is used to provide additional info
 /// about the UEFI implementation.
-pub const PROPERTIES_TABLE_GUID: Guid = Guid::from_values(
-    0x880aaca3,
-    0x4adc,
-    0x4a04,
-    0x9079,
-    [0xb7, 0x47, 0x34, 0x08, 0x25, 0xe5],
-);
+pub const PROPERTIES_TABLE_GUID: Guid =
+    Guid::from_values(0x880aaca3, 0x4adc, 0x4a04, 0x9079, 0xb747340825e5);
 
 /// This table contains additional information about the UEFI implementation.
 #[repr(C)]
@@ -102,65 +73,30 @@ bitflags! {
 /// Hand-off Blocks are used to pass data from the early pre-UEFI environment to the UEFI drivers.
 ///
 /// Most OS loaders or applications should not mess with this.
-pub const HAND_OFF_BLOCK_LIST_GUID: Guid = Guid::from_values(
-    0x7739f24c,
-    0x93d7,
-    0x11d4,
-    0x9a3a,
-    [0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d],
-);
+pub const HAND_OFF_BLOCK_LIST_GUID: Guid =
+    Guid::from_values(0x7739f24c, 0x93d7, 0x11d4, 0x9a3a, 0x0090273fc14d);
 
 /// Table used in the early boot environment to record memory ranges.
-pub const MEMORY_TYPE_INFORMATION_GUID: Guid = Guid::from_values(
-    0x4c19049f,
-    0x4137,
-    0x4dd3,
-    0x9c10,
-    [0x8b, 0x97, 0xa8, 0x3f, 0xfd, 0xfa],
-);
+pub const MEMORY_TYPE_INFORMATION_GUID: Guid =
+    Guid::from_values(0x4c19049f, 0x4137, 0x4dd3, 0x9c10, 0x8b97a83ffdfa);
 
 /// Used to identify Hand-off Blocks which store
 /// status codes reported during the pre-UEFI environment.
-pub const MEMORY_STATUS_CODE_RECORD_GUID: Guid = Guid::from_values(
-    0x60cc026,
-    0x4c0d,
-    0x4dda,
-    0x8f41,
-    [0x59, 0x5f, 0xef, 0x00, 0xa5, 0x02],
-);
+pub const MEMORY_STATUS_CODE_RECORD_GUID: Guid =
+    Guid::from_values(0x60cc026, 0x4c0d, 0x4dda, 0x8f41, 0x595fef00a502);
 
 /// Table which provides Driver eXecution Environment services.
-pub const DXE_SERVICES_GUID: Guid = Guid::from_values(
-    0x5ad34ba,
-    0x6f02,
-    0x4214,
-    0x952e,
-    [0x4d, 0xa0, 0x39, 0x8e, 0x2b, 0xb9],
-);
+pub const DXE_SERVICES_GUID: Guid =
+    Guid::from_values(0x5ad34ba, 0x6f02, 0x4214, 0x952e, 0x4da0398e2bb9);
 
 /// LZMA-compressed filesystem.
-pub const LZMA_COMPRESS_GUID: Guid = Guid::from_values(
-    0xee4e5898,
-    0x3914,
-    0x4259,
-    0x9d6e,
-    [0xdc, 0x7b, 0xd7, 0x94, 0x03, 0xcf],
-);
+pub const LZMA_COMPRESS_GUID: Guid =
+    Guid::from_values(0xee4e5898, 0x3914, 0x4259, 0x9d6e, 0xdc7bd79403cf);
 
 /// A custom compressed filesystem used by the Tiano UEFI implementation.
-pub const TIANO_COMPRESS_GUID: Guid = Guid::from_values(
-    0xa31280ad,
-    0x481e,
-    0x41b6,
-    0x95e8,
-    [0x12, 0x7f, 0x4c, 0x98, 0x47, 0x79],
-);
+pub const TIANO_COMPRESS_GUID: Guid =
+    Guid::from_values(0xa31280ad, 0x481e, 0x41b6, 0x95e8, 0x127f4c984779);
 
 /// Pointer to the debug image info table.
-pub const DEBUG_IMAGE_INFO_GUID: Guid = Guid::from_values(
-    0x49152e77,
-    0x1ada,
-    0x4764,
-    0xb7a2,
-    [0x7a, 0xfe, 0xfe, 0xd9, 0x5e, 0x8b],
-);
+pub const DEBUG_IMAGE_INFO_GUID: Guid =
+    Guid::from_values(0x49152e77, 0x1ada, 0x4764, 0xb7a2, 0x7afefed95e8b);

src/table/runtime.rs

@@ -14,7 +14,6 @@ use core::fmt::{Debug, Formatter};
 use core::mem;
 use core::mem::MaybeUninit;
 use core::{fmt, ptr};
-
 /// Contains pointers to all of the runtime services.
 ///
 /// This table, and the function pointers it contains are valid
@@ -503,7 +502,7 @@ newtype_enum! {
             0x93ca,
             0x11d2,
             0xaa0d,
-            [0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c],
+            0x00e098032b8c,
         ),
 
         /// Used to access EFI signature database variables.
@@ -512,7 +511,7 @@ newtype_enum! {
             0x3d3a,
             0x4596,
             0xa3bc,
-            [0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f],
+            0xdad00e67656f,
         ),
     }
 }

uefi-macros/src/lib.rs

@@ -73,17 +73,7 @@ pub fn unsafe_guid(args: TokenStream, input: TokenStream) -> TokenStream {
     let time_mid = next_guid_int(16) as u16;
     let time_high_and_version = next_guid_int(16) as u16;
     let clock_seq_and_variant = next_guid_int(16) as u16;
-    let node_64 = next_guid_int(48);
-
-    // Convert the node ID to an array of bytes to comply with Guid::from_values expectations
-    let node = [
-        (node_64 >> 40) as u8,
-        ((node_64 >> 32) % 0x100) as u8,
-        ((node_64 >> 24) % 0x100) as u8,
-        ((node_64 >> 16) % 0x100) as u8,
-        ((node_64 >> 8) % 0x100) as u8,
-        (node_64 % 0x100) as u8,
-    ];
+    let node = next_guid_int(48);
 
     // At this point, we know everything we need to implement Identify
     let ident = type_definition.ident.clone();
@@ -97,7 +87,7 @@ pub fn unsafe_guid(args: TokenStream, input: TokenStream) -> TokenStream {
                 #time_mid,
                 #time_high_and_version,
                 #clock_seq_and_variant,
-                [#(#node),*],
+                #node,
             );
         }
     });

uefi-test-runner/src/runtime/vars.rs

@@ -15,7 +15,7 @@ fn test_variables(rt: &RuntimeServices) {
         0xe187,
         0x497e,
         0xae77,
-        [0x5b, 0xd8, 0xb0, 0xe0, 0x97, 0x03],
+        0x5bd8b0e09703,
     ));
 
     info!("Testing set_variable");