From 58ed2cbd789e8dc70415fb62d3cb6d26382bd115 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sat, 10 Jun 2023 12:36:47 -0400 Subject: [PATCH] uefi: Remove some uses of MaybeUninit in BootServices While `MaybeUninit` is useful to avoid unnecessarily initializing large buffers, it's not needed for small values like single pointers and integers. For `Event`, `Handle`, and `ProtocolSearchKey`, which internally contain a `NonNull` pointer, we can wrap them in an `Option` and still have the same layout as a raw pointer. The initial value can then be `None` rather than `MaybeUninit`. This has the nice side effect of removing a little bit of `unsafe` code. --- uefi/src/table/boot.rs | 77 ++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/uefi/src/table/boot.rs b/uefi/src/table/boot.rs index 1a1672026..e9ff3e50a 100644 --- a/uefi/src/table/boot.rs +++ b/uefi/src/table/boot.rs @@ -122,7 +122,7 @@ pub struct BootServices { notify_tpl: Tpl, notify_func: Option, notify_ctx: Option>, - out_event: *mut Event, + out_event: *mut Option, ) -> Status, set_timer: unsafe extern "efiapi" fn(event: Event, ty: u32, trigger_time: u64) -> Status, wait_for_event: unsafe extern "efiapi" fn( @@ -159,7 +159,7 @@ pub struct BootServices { register_protocol_notify: extern "efiapi" fn( protocol: &Guid, event: Event, - registration: *mut ProtocolSearchKey, + registration: *mut Option, ) -> Status, locate_handle: unsafe extern "efiapi" fn( search_ty: i32, @@ -171,7 +171,7 @@ pub struct BootServices { locate_device_path: unsafe extern "efiapi" fn( proto: &Guid, device_path: &mut *const FfiDevicePath, - out_handle: &mut MaybeUninit, + out_handle: &mut Option, ) -> Status, install_configuration_table: extern "efiapi" fn(guid_entry: &Guid, table_ptr: *const c_void) -> Status, @@ -183,7 +183,7 @@ pub struct BootServices { device_path: *const FfiDevicePath, source_buffer: *const u8, source_size: usize, - image_handle: &mut MaybeUninit, + image_handle: &mut Option, ) -> Status, start_image: unsafe extern "efiapi" fn( image_handle: Handle, @@ -276,7 +276,7 @@ pub struct BootServices { notify_fn: Option, notify_ctx: Option>, event_group: Option>, - out_event: *mut Event, + out_event: *mut Option, ) -> Status, } @@ -519,18 +519,14 @@ impl BootServices { notify_fn: Option, notify_ctx: Option>, ) -> Result { - // Prepare storage for the output Event - let mut event = MaybeUninit::::uninit(); + let mut event = None; // Now we're ready to call UEFI - (self.create_event)( - event_ty, - notify_tpl, - notify_fn, - notify_ctx, - event.as_mut_ptr(), - ) - .to_result_with_val(|| event.assume_init()) + (self.create_event)(event_ty, notify_tpl, notify_fn, notify_ctx, &mut event) + .to_result_with_val( + // OK to unwrap: event is non-null for Status::SUCCESS. + || event.unwrap(), + ) } /// Creates a new `Event` of type `event_type`. The event's notification function, context, @@ -584,7 +580,7 @@ impl BootServices { return Err(Status::UNSUPPORTED.into()); } - let mut event = MaybeUninit::::uninit(); + let mut event = None; (self.create_event_ex)( event_type, @@ -592,9 +588,12 @@ impl BootServices { notify_fn, notify_ctx, event_group, - event.as_mut_ptr(), + &mut event, + ) + .to_result_with_val( + // OK to unwrap: event is non-null for Status::SUCCESS. + || event.unwrap(), ) - .to_result_with_val(|| event.assume_init()) } /// Sets the trigger for `EventType::TIMER` event. @@ -649,18 +648,17 @@ impl BootServices { /// * [`uefi::Status::UNSUPPORTED`] pub fn wait_for_event(&self, events: &mut [Event]) -> Result> { let (number_of_events, events) = (events.len(), events.as_mut_ptr()); - let mut index = MaybeUninit::::uninit(); - unsafe { (self.wait_for_event)(number_of_events, events, index.as_mut_ptr()) } - .to_result_with( - || unsafe { index.assume_init() }, - |s| { - if s == Status::INVALID_PARAMETER { - unsafe { Some(index.assume_init()) } - } else { - None - } - }, - ) + let mut index = 0; + unsafe { (self.wait_for_event)(number_of_events, events, &mut index) }.to_result_with( + || index, + |s| { + if s == Status::INVALID_PARAMETER { + Some(index) + } else { + None + } + }, + ) } /// Place 'event' in the signaled stated. If 'event' is already in the signaled state, @@ -839,14 +837,15 @@ impl BootServices { protocol: &Guid, event: Event, ) -> Result<(Event, SearchType)> { - let mut key: MaybeUninit = MaybeUninit::uninit(); + let mut key = None; // Safety: we clone `event` a couple times, but there will be only one left once we return. - unsafe { (self.register_protocol_notify)(protocol, event.unsafe_clone(), key.as_mut_ptr()) } + unsafe { (self.register_protocol_notify)(protocol, event.unsafe_clone(), &mut key) } // Safety: as long as this call is successful, `key` will be valid. .to_result_with_val(|| unsafe { ( event.unsafe_clone(), - SearchType::ByRegisterNotify(key.assume_init()), + // OK to unwrap: key is non-null for Status::SUCCESS. + SearchType::ByRegisterNotify(key.unwrap()), ) }) } @@ -918,13 +917,14 @@ impl BootServices { &self, device_path: &mut &DevicePath, ) -> Result { - let mut handle = MaybeUninit::uninit(); + let mut handle = None; let mut device_path_ptr = device_path.as_ffi_ptr(); unsafe { (self.locate_device_path)(&P::GUID, &mut device_path_ptr, &mut handle) .to_result_with_val(|| { *device_path = DevicePath::from_ffi_ptr(device_path_ptr); - handle.assume_init() + // OK to unwrap: handle is non-null for Status::SUCCESS. + handle.unwrap() }) } } @@ -1037,7 +1037,7 @@ impl BootServices { } }; - let mut image_handle = MaybeUninit::uninit(); + let mut image_handle = None; unsafe { (self.load_image)( boot_policy, @@ -1047,7 +1047,10 @@ impl BootServices { source_size, &mut image_handle, ) - .to_result_with_val(|| image_handle.assume_init()) + .to_result_with_val( + // OK to unwrap: image handle is non-null for Status::SUCCESS. + || image_handle.unwrap(), + ) } }