From 48e8f25a0ccb4d71800bff286cbadff737393173 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Fri, 10 Mar 2023 13:36:42 -0500 Subject: [PATCH] uefi: Drop `'boot` lifetime from Output protocol The `Output` protocol has an internal pointer to an `OutputData` object. This was represented as a reference with the `'boot` lifetime, but this isn't quite right since the actual lifetime could be shorter than boot services. Furthermore, in several places the lifetime parameter was set to `'static`, which is definitely not correct. Fix by dropping the lifetime parameter, and instead just use a raw pointer. This is more in line with how we've implemented other protocols. --- CHANGELOG.md | 1 + uefi-test-runner/src/proto/mod.rs | 4 +--- uefi/src/logger.rs | 2 +- uefi/src/proto/console/text/output.rs | 34 ++++++++++++++++----------- uefi/src/table/system.rs | 4 ++-- 5 files changed, 25 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47edfacf4..a6976186c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - `HandleBuffer` and `ProtocolsPerHandle` now implement `Deref`. The `HandleBuffer::handles` and `ProtocolsPerHandle::protocols` methods have been deprecated. +- Removed `'boot` lifetime from the `Output` protocol. ## uefi-macros - [Unreleased] diff --git a/uefi-test-runner/src/proto/mod.rs b/uefi-test-runner/src/proto/mod.rs index 4b3c5a86f..fdf412286 100644 --- a/uefi-test-runner/src/proto/mod.rs +++ b/uefi-test-runner/src/proto/mod.rs @@ -33,10 +33,8 @@ pub fn test(image: Handle, st: &mut SystemTable) { } fn find_protocol(bt: &BootServices) { - type SearchedProtocol<'boot> = proto::console::text::Output<'boot>; - let handles = bt - .find_handles::() + .find_handles::() .expect("Failed to retrieve list of handles"); assert!( diff --git a/uefi/src/logger.rs b/uefi/src/logger.rs index a3bf1e27b..893c48b9e 100644 --- a/uefi/src/logger.rs +++ b/uefi/src/logger.rs @@ -23,7 +23,7 @@ use core::ptr::NonNull; /// `disable` method before exiting UEFI boot services in order to prevent /// undefined behaviour from inadvertent logging. pub struct Logger { - writer: Option>>, + writer: Option>, } impl Logger { diff --git a/uefi/src/proto/console/text/output.rs b/uefi/src/proto/console/text/output.rs index 8e6b5addd..b3492bcf0 100644 --- a/uefi/src/proto/console/text/output.rs +++ b/uefi/src/proto/console/text/output.rs @@ -22,7 +22,7 @@ use core::fmt::{Debug, Formatter}; /// [`BootServices`]: crate::table::boot::BootServices#accessing-protocols #[repr(C)] #[unsafe_protocol("387477c2-69c7-11d2-8e39-00a0c969723b")] -pub struct Output<'boot> { +pub struct Output { reset: extern "efiapi" fn(this: &Output, extended: bool) -> Status, output_string: unsafe extern "efiapi" fn(this: &Output, string: *const Char16) -> Status, test_string: unsafe extern "efiapi" fn(this: &Output, string: *const Char16) -> Status, @@ -37,10 +37,10 @@ pub struct Output<'boot> { clear_screen: extern "efiapi" fn(this: &mut Output) -> Status, set_cursor_position: extern "efiapi" fn(this: &mut Output, column: usize, row: usize) -> Status, enable_cursor: extern "efiapi" fn(this: &mut Output, visible: bool) -> Status, - data: &'boot OutputData, + data: *const OutputData, } -impl<'boot> Output<'boot> { +impl Output { /// Resets and clears the text output device hardware. pub fn reset(&mut self, extended: bool) -> Result { (self.reset)(self, extended).into() @@ -85,8 +85,8 @@ impl<'boot> Output<'boot> { /// Returns an iterator of all supported text modes. // TODO: Bring back impl Trait once the story around bounds improves - pub fn modes<'out>(&'out mut self) -> OutputModeIter<'out, 'boot> { - let max = self.data.max_mode as usize; + pub fn modes(&mut self) -> OutputModeIter<'_> { + let max = self.data().max_mode as usize; OutputModeIter { output: self, current: 0, @@ -111,7 +111,7 @@ impl<'boot> Output<'boot> { /// Returns the current text mode. pub fn current_mode(&self) -> Result> { - match self.data.mode { + match self.data().mode { -1 => Ok(None), n if n >= 0 => { let index = n as usize; @@ -130,7 +130,7 @@ impl<'boot> Output<'boot> { /// Returns whether the cursor is currently shown or not. #[must_use] pub const fn cursor_visible(&self) -> bool { - self.data.cursor_visible + self.data().cursor_visible } /// Make the cursor visible or invisible. @@ -144,8 +144,8 @@ impl<'boot> Output<'boot> { /// Returns the column and row of the cursor. #[must_use] pub const fn cursor_position(&self) -> (usize, usize) { - let column = self.data.cursor_column; - let row = self.data.cursor_row; + let column = self.data().cursor_column; + let row = self.data().cursor_row; (column as usize, row as usize) } @@ -169,9 +169,15 @@ impl<'boot> Output<'boot> { let attr = ((bgc & 0x7) << 4) | (fgc & 0xF); (self.set_attribute)(self, attr).into() } + + /// Get a reference to `OutputData`. The lifetime of the reference is tied + /// to `self`. + const fn data(&self) -> &OutputData { + unsafe { &*self.data } + } } -impl<'boot> fmt::Write for Output<'boot> { +impl fmt::Write for Output { fn write_str(&mut self, s: &str) -> fmt::Result { // Allocate a small buffer on the stack. const BUF_SIZE: usize = 128; @@ -222,7 +228,7 @@ impl<'boot> fmt::Write for Output<'boot> { } } -impl<'boot> Debug for Output<'boot> { +impl Debug for Output { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { f.debug_struct("Output") .field("reset (fn ptr)", &(self.reset as *const u64)) @@ -282,13 +288,13 @@ impl OutputMode { } /// An iterator of the text modes (possibly) supported by a device. -pub struct OutputModeIter<'out, 'boot: 'out> { - output: &'out mut Output<'boot>, +pub struct OutputModeIter<'out> { + output: &'out mut Output, current: usize, max: usize, } -impl<'out, 'boot> Iterator for OutputModeIter<'out, 'boot> { +impl<'out> Iterator for OutputModeIter<'out> { type Item = OutputMode; fn next(&mut self) -> Option { diff --git a/uefi/src/table/system.rs b/uefi/src/table/system.rs index 7fbcdcea3..e78b38035 100644 --- a/uefi/src/table/system.rs +++ b/uefi/src/table/system.rs @@ -346,9 +346,9 @@ struct SystemTableImpl { stdin_handle: Handle, stdin: *mut text::Input, stdout_handle: Handle, - stdout: *mut text::Output<'static>, + stdout: *mut text::Output, stderr_handle: Handle, - stderr: *mut text::Output<'static>, + stderr: *mut text::Output, /// Runtime services table. runtime: &'static RuntimeServices, /// Boot services table.