Merge pull request #1328 from rust-osdev/revert-1327-bishop-dp-text-no-bs

Revert "uefi: Drop BootServices arg from device path <-> text conversions"

Author: phip1611

uefi-test-runner/examples/loaded_image.rs

@@ -49,6 +49,7 @@ fn print_image_path(boot_services: &BootServices) -> Result {
         loaded_image.file_path().expect("File path is not set");
     let image_device_path_text = device_path_to_text
         .convert_device_path_to_text(
+            boot_services,
             image_device_path,
             DisplayOnly(true),
             AllowShortcuts(false),

uefi-test-runner/src/proto/device_path.rs

@@ -43,7 +43,7 @@ pub fn test(bt: &BootServices) {
             );
 
             let text = device_path_to_text
-                .convert_device_node_to_text(path, DisplayOnly(true), AllowShortcuts(false))
+                .convert_device_node_to_text(bt, path, DisplayOnly(true), AllowShortcuts(false))
                 .expect("Failed to convert device path to text");
             let text = &*text;
             info!("path name: {text}");
@@ -81,7 +81,7 @@ pub fn test(bt: &BootServices) {
 
         let path_components = device_path
             .node_iter()
-            .map(|node| node.to_string(DisplayOnly(false), AllowShortcuts(false)))
+            .map(|node| node.to_string(bt, DisplayOnly(false), AllowShortcuts(false)))
             .map(|str| str.unwrap().to_string())
             .collect::<Vec<_>>();
 
@@ -107,7 +107,7 @@ pub fn test(bt: &BootServices) {
 
         // Test that to_string works for device_paths
         let path = device_path
-            .to_string(DisplayOnly(false), AllowShortcuts(false))
+            .to_string(bt, DisplayOnly(false), AllowShortcuts(false))
             .unwrap()
             .to_string();
 

uefi/CHANGELOG.md

@@ -23,8 +23,6 @@ details of a significant change to the API in this release.
 
 ## Changed
 - **Breaking:** `uefi::helpers::init` no longer takes an argument.
-- **Breaking:** The conversion functions between device paths and text no longer
-  take a `BootServices` argument. The global system table is used instead.
 - The lifetime of the `SearchType` returned from
   `BootServices::register_protocol_notify` is now tied to the protocol GUID.
   The old `MemoryMap` was renamed to `MemoryMapOwned`.

uefi/src/proto/device_path/mod.rs

@@ -90,8 +90,10 @@ use ptr_meta::Pointee;
 
 #[cfg(feature = "alloc")]
 use {
-    crate::boot::{self, OpenProtocolAttributes, OpenProtocolParams, ScopedProtocol, SearchType},
     crate::proto::device_path::text::{AllowShortcuts, DevicePathToText, DisplayOnly},
+    crate::table::boot::{
+        BootServices, OpenProtocolAttributes, OpenProtocolParams, ScopedProtocol, SearchType,
+    },
     crate::{CString16, Identify},
     alloc::borrow::ToOwned,
     alloc::boxed::Box,
@@ -230,13 +232,14 @@ impl DevicePathNode {
     #[cfg(feature = "alloc")]
     pub fn to_string(
         &self,
+        bs: &BootServices,
         display_only: DisplayOnly,
         allow_shortcuts: AllowShortcuts,
     ) -> Result<CString16, DevicePathToTextError> {
-        let to_text_protocol = open_text_protocol()?;
+        let to_text_protocol = open_text_protocol(bs)?;
 
         to_text_protocol
-            .convert_device_node_to_text(self, display_only, allow_shortcuts)
+            .convert_device_node_to_text(bs, self, display_only, allow_shortcuts)
             .map(|pool_string| {
                 let cstr16 = &*pool_string;
                 // Another allocation; pool string is dropped. This overhead
@@ -481,13 +484,14 @@ impl DevicePath {
     #[cfg(feature = "alloc")]
     pub fn to_string(
         &self,
+        bs: &BootServices,
         display_only: DisplayOnly,
         allow_shortcuts: AllowShortcuts,
     ) -> Result<CString16, DevicePathToTextError> {
-        let to_text_protocol = open_text_protocol()?;
+        let to_text_protocol = open_text_protocol(bs)?;
 
         to_text_protocol
-            .convert_device_path_to_text(self, display_only, allow_shortcuts)
+            .convert_device_path_to_text(bs, self, display_only, allow_shortcuts)
             .map(|pool_string| {
                 let cstr16 = &*pool_string;
                 // Another allocation; pool string is dropped. This overhead
@@ -874,17 +878,20 @@ impl core::error::Error for DevicePathToTextError {
 /// Helper function to open the [`DevicePathToText`] protocol using the boot
 /// services.
 #[cfg(feature = "alloc")]
-fn open_text_protocol() -> Result<ScopedProtocol<DevicePathToText>, DevicePathToTextError> {
-    let &handle = boot::locate_handle_buffer(SearchType::ByProtocol(&DevicePathToText::GUID))
+fn open_text_protocol(
+    bs: &BootServices,
+) -> Result<ScopedProtocol<DevicePathToText>, DevicePathToTextError> {
+    let &handle = bs
+        .locate_handle_buffer(SearchType::ByProtocol(&DevicePathToText::GUID))
         .map_err(DevicePathToTextError::CantLocateHandleBuffer)?
         .first()
         .ok_or(DevicePathToTextError::NoHandle)?;
 
     unsafe {
-        boot::open_protocol::<DevicePathToText>(
+        bs.open_protocol::<DevicePathToText>(
             OpenProtocolParams {
                 handle,
-                agent: boot::image_handle(),
+                agent: bs.image_handle(),
                 controller: None,
             },
             OpenProtocolAttributes::GetProtocol,

uefi/src/proto/device_path/text.rs

@@ -10,9 +10,9 @@
 
 use crate::proto::device_path::{DevicePath, DevicePathNode};
 use crate::proto::unsafe_protocol;
-use crate::{boot, CStr16, Char16, Result, Status};
+use crate::table::boot::BootServices;
+use crate::{CStr16, Char16, Result, Status};
 use core::ops::Deref;
-use core::ptr::NonNull;
 use uefi_raw::protocol::device_path::{DevicePathFromTextProtocol, DevicePathToTextProtocol};
 
 /// This struct is a wrapper of `display_only` parameter
@@ -42,27 +42,36 @@ pub struct AllowShortcuts(pub bool);
 /// Wrapper for a string internally allocated from
 /// UEFI boot services memory.
 #[derive(Debug)]
-pub struct PoolString(NonNull<Char16>);
+pub struct PoolString<'a> {
+    boot_services: &'a BootServices,
+    text: *const Char16,
+}
 
-impl PoolString {
-    fn new(text: *const Char16) -> Result<Self> {
-        NonNull::new(text.cast_mut())
-            .map(Self)
-            .ok_or(Status::OUT_OF_RESOURCES.into())
+impl<'a> PoolString<'a> {
+    fn new(boot_services: &'a BootServices, text: *const Char16) -> Result<Self> {
+        if text.is_null() {
+            Err(Status::OUT_OF_RESOURCES.into())
+        } else {
+            Ok(Self {
+                boot_services,
+                text,
+            })
+        }
     }
 }
 
-impl Deref for PoolString {
+impl<'a> Deref for PoolString<'a> {
     type Target = CStr16;
 
     fn deref(&self) -> &Self::Target {
-        unsafe { CStr16::from_ptr(self.0.as_ptr()) }
+        unsafe { CStr16::from_ptr(self.text) }
     }
 }
 
-impl Drop for PoolString {
+impl Drop for PoolString<'_> {
     fn drop(&mut self) {
-        unsafe { boot::free_pool(self.0.cast()) }.expect("Failed to free pool [{addr:#?}]");
+        let addr = self.text as *mut u8;
+        unsafe { self.boot_services.free_pool(addr) }.expect("Failed to free pool [{addr:#?}]");
     }
 }
 
@@ -82,20 +91,21 @@ impl DevicePathToText {
     /// memory for the conversion.
     ///
     /// [`OUT_OF_RESOURCES`]: Status::OUT_OF_RESOURCES
-    pub fn convert_device_node_to_text(
+    pub fn convert_device_node_to_text<'boot>(
         &self,
+        boot_services: &'boot BootServices,
         device_node: &DevicePathNode,
         display_only: DisplayOnly,
         allow_shortcuts: AllowShortcuts,
-    ) -> Result<PoolString> {
+    ) -> Result<PoolString<'boot>> {
         let text_device_node = unsafe {
             (self.0.convert_device_node_to_text)(
                 device_node.as_ffi_ptr().cast(),
                 display_only.0,
                 allow_shortcuts.0,
             )
         };
-        PoolString::new(text_device_node.cast())
+        PoolString::new(boot_services, text_device_node.cast())
     }
 
     /// Convert a device path to its text representation.
@@ -104,20 +114,21 @@ impl DevicePathToText {
     /// memory for the conversion.
     ///
     /// [`OUT_OF_RESOURCES`]: Status::OUT_OF_RESOURCES
-    pub fn convert_device_path_to_text(
+    pub fn convert_device_path_to_text<'boot>(
         &self,
+        boot_services: &'boot BootServices,
         device_path: &DevicePath,
         display_only: DisplayOnly,
         allow_shortcuts: AllowShortcuts,
-    ) -> Result<PoolString> {
+    ) -> Result<PoolString<'boot>> {
         let text_device_path = unsafe {
             (self.0.convert_device_path_to_text)(
                 device_path.as_ffi_ptr().cast(),
                 display_only.0,
                 allow_shortcuts.0,
             )
         };
-        PoolString::new(text_device_path.cast())
+        PoolString::new(boot_services, text_device_path.cast())
     }
 }