Merge pull request #1983 from rust-osdev/doc-dp-builder

uefi: minor improvements to device path protocol code

Author: phip1611

uefi/src/proto/device_path/build.rs

@@ -99,7 +99,7 @@ impl<'a> DevicePathBuilder<'a> {
     /// Add a node to the device path.
     ///
     /// An error will be returned if an [`END_ENTIRE`] node is passed to
-    /// this function, as that node will be added when `finalize` is
+    /// this function, as that node will be added when [`Self::finalize`] is
     /// called.
     ///
     /// [`END_ENTIRE`]: uefi::proto::device_path::DeviceSubType::END_ENTIRE
@@ -150,6 +150,7 @@ impl<'a> DevicePathBuilder<'a> {
     }
 }
 
+/// Reference to the backup storage for [`DevicePathBuilder`]
 #[derive(Debug)]
 enum BuilderStorage<'a> {
     Buf {
@@ -246,11 +247,6 @@ mod tests {
     use crate::proto::device_path::messaging::{
         Ipv4AddressOrigin, IscsiLoginOptions, IscsiProtocol, RestServiceAccessMode, RestServiceType,
     };
-    use core::slice;
-
-    const fn path_to_bytes(path: &DevicePath) -> &[u8] {
-        unsafe { slice::from_raw_parts(path.as_ffi_ptr().cast::<u8>(), size_of_val(path)) }
-    }
 
     /// Test building an ACPI ADR node.
     #[test]
@@ -267,10 +263,8 @@ mod tests {
         let node: &crate::proto::device_path::acpi::Adr =
             path.node_iter().next().unwrap().try_into().unwrap();
         assert_eq!(node.adr().iter().collect::<Vec<_>>(), [1, 2]);
-
-        let bytes = path_to_bytes(path);
         #[rustfmt::skip]
-        assert_eq!(bytes, [
+        assert_eq!(path.as_bytes(), [
             // ACPI ADR node
             0x02, 0x03, 0x0c, 0x00,
             // Values
@@ -308,9 +302,8 @@ mod tests {
         assert_eq!(node.uid_str(), b"bc\0");
         assert_eq!(node.cid_str(), b"def\0");
 
-        let bytes = path_to_bytes(path);
         #[rustfmt::skip]
-        assert_eq!(bytes, [
+        assert_eq!(path.as_bytes(), [
             // ACPI Expanded node
             0x02, 0x02, 0x19, 0x00,
             // HID
@@ -365,9 +358,8 @@ mod tests {
         assert_eq!(node.vendor_guid_and_data().unwrap().0, vendor_guid);
         assert_eq!(node.vendor_guid_and_data().unwrap().1, &[1, 2, 3, 4, 5]);
 
-        let bytes = path_to_bytes(path);
         #[rustfmt::skip]
-        assert_eq!(bytes, [
+        assert_eq!(path.as_bytes(), [
             // Messaging REST Service node.
             0x03, 0x21, 0x06, 0x00,
             // Type and access mode
@@ -428,27 +420,32 @@ mod tests {
     /// from the UEFI Specification.
     #[test]
     fn test_fibre_channel_ex_device_path_example() -> Result<(), BuildError> {
-        // Arbitrarily choose this test to use a statically-sized
-        // buffer, just to make sure that code path is tested.
-        let mut buf = [MaybeUninit::uninit(); 256];
-        let path = DevicePathBuilder::with_buf(&mut buf)
-            .push(&acpi::Acpi {
+        let nodes: &[&dyn BuildNode] = &[
+            &acpi::Acpi {
                 hid: 0x41d0_0a03,
                 uid: 0x0000_0000,
-            })?
-            .push(&hardware::Pci {
+            },
+            &hardware::Pci {
                 function: 0x00,
                 device: 0x1f,
-            })?
-            .push(&messaging::FibreChannelEx {
+            },
+            &messaging::FibreChannelEx {
                 world_wide_name: [0, 1, 2, 3, 4, 5, 6, 7],
                 logical_unit_number: [0, 1, 2, 3, 4, 5, 6, 7],
-            })?
+            },
+        ];
+
+        // Arbitrarily choose this test to use a statically-sized
+        // buffer, just to make sure that code path is tested.
+        let mut buf = [MaybeUninit::uninit(); 256];
+        let path1 = DevicePathBuilder::with_buf(&mut buf)
+            .push(nodes[0])?
+            .push(nodes[1])?
+            .push(nodes[2])?
             .finalize()?;
 
-        let bytes = path_to_bytes(path);
         #[rustfmt::skip]
-        assert_eq!(bytes, [
+        const EXPECTED: [u8; 46] = [
             // ACPI node
             0x02, 0x01, 0x0c, 0x00,
             // HID
@@ -477,7 +474,9 @@ mod tests {
 
             // End-entire node
             0x7f, 0xff, 0x04, 0x00,
-        ]);
+        ];
+
+        assert_eq!(path1.as_bytes(), EXPECTED);
 
         Ok(())
     }
@@ -532,9 +531,8 @@ mod tests {
             })?
             .finalize()?;
 
-        let bytes = path_to_bytes(path);
         #[rustfmt::skip]
-        assert_eq!(bytes, [
+        assert_eq!(path.as_bytes(), [
             // ACPI node
             0x02, 0x01, 0x0c, 0x00,
             // HID

uefi/src/proto/device_path/mod.rs

@@ -75,6 +75,8 @@
 //!   other types in this module are DSTs, so pointers to the type are
 //!   "fat" and not suitable for FFI.
 //!
+//! * [`PoolDevicePath`] is an owned device path on the UEFI heap.
+//!
 //! All of these types use a packed layout and may appear on any byte
 //! boundary.
 //!
@@ -134,6 +136,10 @@ opaque_type! {
 }
 
 /// Device path allocated from UEFI pool memory.
+///
+/// Please note that this differs from <code>Box<[DevicePath]></code>. Although
+/// both represent owned values, a <code>Box<[DevicePath]></code> is on the Rust
+/// heap which may or may not be backed by the UEFI heap.
 #[derive(Debug)]
 pub struct PoolDevicePath(pub(crate) PoolAllocation);
 
@@ -408,11 +414,15 @@ impl DevicePathInstance {
     }
 
     /// Returns a boxed copy of that value.
+    ///
+    /// The semantics slightly differs from a [`PoolDevicePath`] but is
+    /// generally more idiomatic to use.
     #[cfg(feature = "alloc")]
     #[must_use]
     pub fn to_boxed(&self) -> Box<Self> {
         let data = self.data.to_owned();
         let data = data.into_boxed_slice();
+        // SAFETY: This is safe as a DevicePath has the same layout.
         unsafe { mem::transmute(data) }
     }
 }
@@ -595,12 +605,16 @@ impl DevicePath {
         &self.data
     }
 
-    /// Returns a boxed copy of that value.
+    /// Returns a boxed copy of that value on the Rust heap.
+    ///
+    /// The semantics slightly differs from a [`PoolDevicePath`] but is
+    /// generally more idiomatic to use.
     #[cfg(feature = "alloc")]
     #[must_use]
     pub fn to_boxed(&self) -> Box<Self> {
         let data = self.data.to_owned();
         let data = data.into_boxed_slice();
+        // SAFETY: This is safe as a DevicePath has the same layout.
         unsafe { mem::transmute(data) }
     }