From e60e9902e656150678e6e22fcf53fa57e540c059 Mon Sep 17 00:00:00 2001 From: Mathieu Gravel Date: Mon, 29 Jul 2024 13:54:18 -0700 Subject: [PATCH] UEFI HID DXE V2 to improve Idiomatic Rust patterns (#504) - use more rust idiomatic naming convention - use `let else` syntax for better readability - Organize import with the same logic in every file. 1. crate from rust 2. crate from crate.io 3. crate from Microsoft 4. use from the same crate - [ ] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested Test that the keyboard was still working. ## Integration Instructions N/A --- HidPkg/UefiHidDxeV2/src/boot_services.rs | 1 + HidPkg/UefiHidDxeV2/src/hid.rs | 14 +- HidPkg/UefiHidDxeV2/src/hid_io.rs | 36 +- .../src/{keyboard/mod.rs => keyboard.rs} | 97 +++-- HidPkg/UefiHidDxeV2/src/keyboard/key_queue.rs | 92 ++--- .../src/keyboard/simple_text_in.rs | 77 ++-- .../src/keyboard/simple_text_in_ex.rs | 330 ++++++++---------- HidPkg/UefiHidDxeV2/src/lib.rs | 8 +- HidPkg/UefiHidDxeV2/src/main.rs | 3 +- .../src/{pointer/mod.rs => pointer.rs} | 9 +- .../src/pointer/absolute_pointer.rs | 50 ++- 11 files changed, 337 insertions(+), 380 deletions(-) rename HidPkg/UefiHidDxeV2/src/{keyboard/mod.rs => keyboard.rs} (93%) rename HidPkg/UefiHidDxeV2/src/{pointer/mod.rs => pointer.rs} (97%) diff --git a/HidPkg/UefiHidDxeV2/src/boot_services.rs b/HidPkg/UefiHidDxeV2/src/boot_services.rs index cd19af0518..4460913c0e 100644 --- a/HidPkg/UefiHidDxeV2/src/boot_services.rs +++ b/HidPkg/UefiHidDxeV2/src/boot_services.rs @@ -11,6 +11,7 @@ //! SPDX-License-Identifier: BSD-2-Clause-Patent //! use core::{ffi::c_void, fmt::Debug, sync::atomic::AtomicPtr}; + #[cfg(test)] use mockall::automock; use r_efi::efi; diff --git a/HidPkg/UefiHidDxeV2/src/hid.rs b/HidPkg/UefiHidDxeV2/src/hid.rs index 37fbd2dc1d..df3f4cc48a 100644 --- a/HidPkg/UefiHidDxeV2/src/hid.rs +++ b/HidPkg/UefiHidDxeV2/src/hid.rs @@ -41,22 +41,20 @@ //! //! SPDX-License-Identifier: BSD-2-Clause-Patent //! +use alloc::{boxed::Box, vec::Vec}; use core::ffi::c_void; +#[cfg(test)] +use mockall::automock; +use r_efi::efi; +use rust_advanced_logger_dxe::{debugln, DEBUG_ERROR}; + use crate::{ boot_services::UefiBootServices, driver_binding::DriverBinding, hid_io::{HidIo, HidIoFactory, HidReportReceiver}, }; -use alloc::{boxed::Box, vec::Vec}; - -use r_efi::efi; -use rust_advanced_logger_dxe::{debugln, DEBUG_ERROR}; - -#[cfg(test)] -use mockall::automock; - /// This trait defines an abstraction for getting a list of receivers for HID reports. /// /// This is used to specify to a HidFactory how it should instantiate new receivers for HID reports. diff --git a/HidPkg/UefiHidDxeV2/src/hid_io.rs b/HidPkg/UefiHidDxeV2/src/hid_io.rs index 805bbedc8d..1726740709 100644 --- a/HidPkg/UefiHidDxeV2/src/hid_io.rs +++ b/HidPkg/UefiHidDxeV2/src/hid_io.rs @@ -10,18 +10,17 @@ //! //! SPDX-License-Identifier: BSD-2-Clause-Patent //! -use core::{ffi::c_void, slice::from_raw_parts_mut}; - use alloc::{boxed::Box, vec}; +use core::{ffi::c_void, ptr, slice::from_raw_parts_mut}; + +#[cfg(test)] +use mockall::automock; +use r_efi::efi; use hid_io::protocol::HidReportType; use hidparser::ReportDescriptor; -use r_efi::efi; use rust_advanced_logger_dxe::{debugln, DEBUG_ERROR}; -#[cfg(test)] -use mockall::automock; - use crate::boot_services::UefiBootServices; /// Defines an interface to be implemented by logic that wants to receive hid reports. @@ -97,7 +96,7 @@ impl UefiHidIo { controller: efi::Handle, owned: bool, ) -> Result { - let mut hid_io_ptr: *mut hid_io::protocol::Protocol = core::ptr::null_mut(); + let mut hid_io_ptr: *mut hid_io::protocol::Protocol = ptr::null_mut(); let attributes = { if owned { @@ -110,7 +109,7 @@ impl UefiHidIo { let status = boot_services.open_protocol( controller, &hid_io::protocol::GUID as *const efi::Guid as *mut efi::Guid, - core::ptr::addr_of_mut!(hid_io_ptr) as *mut *mut c_void, + ptr::addr_of_mut!(hid_io_ptr) as *mut *mut c_void, agent, controller, attributes, @@ -158,8 +157,8 @@ impl HidIo for UefiHidIo { let mut report_descriptor_size: usize = 0; match (self.hid_io.get_report_descriptor)( self.hid_io, - core::ptr::addr_of_mut!(report_descriptor_size), - core::ptr::null_mut(), + ptr::addr_of_mut!(report_descriptor_size), + ptr::null_mut(), ) { efi::Status::BUFFER_TOO_SMALL => (), efi::Status::SUCCESS => return Err(efi::Status::DEVICE_ERROR), @@ -171,7 +170,7 @@ impl HidIo for UefiHidIo { match (self.hid_io.get_report_descriptor)( self.hid_io, - core::ptr::addr_of_mut!(report_descriptor_size), + ptr::addr_of_mut!(report_descriptor_size), report_descriptor_buffer_ptr as *mut c_void, ) { efi::Status::SUCCESS => (), @@ -227,6 +226,7 @@ impl HidIo for UefiHidIo { mod test { use core::{ ffi::c_void, + ptr, slice::{from_raw_parts, from_raw_parts_mut}, }; @@ -270,7 +270,7 @@ mod test { report_descriptor_size: *mut usize, report_descriptor_buffer: *mut c_void, ) -> efi::Status { - assert_ne!(this, core::ptr::null()); + assert_ne!(this, ptr::null()); unsafe { if *report_descriptor_size < MINIMAL_BOOT_KEYBOARD_REPORT_DESCRIPTOR.len() { *report_descriptor_size = MINIMAL_BOOT_KEYBOARD_REPORT_DESCRIPTOR.len(); @@ -301,10 +301,10 @@ mod test { report_buffer_size: usize, report_buffer: *mut c_void, ) -> efi::Status { - assert_ne!(this, core::ptr::null()); + assert_ne!(this, ptr::null()); assert_eq!(report_type, hid_io::protocol::HidReportType::OutputReport); assert_ne!(report_buffer_size, 0); - assert_ne!(report_buffer, core::ptr::null_mut()); + assert_ne!(report_buffer, ptr::null_mut()); let report_slice = unsafe { from_raw_parts(report_buffer as *mut u8, report_buffer_size) }; @@ -326,8 +326,8 @@ mod test { callback: hid_io::protocol::HidIoReportCallback, context: *mut c_void, ) -> efi::Status { - assert_ne!(this, core::ptr::null()); - assert_ne!(context, core::ptr::null_mut()); + assert_ne!(this, ptr::null()); + assert_ne!(context, ptr::null_mut()); assert!(callback == UefiHidIo::report_callback); callback(TEST_REPORT0.len() as u16, TEST_REPORT0.as_ptr() as *mut c_void, context); @@ -339,7 +339,7 @@ mod test { this: *const hid_io::protocol::Protocol, callback: hid_io::protocol::HidIoReportCallback, ) -> efi::Status { - assert_ne!(this, core::ptr::null()); + assert_ne!(this, ptr::null()); assert!(callback == UefiHidIo::report_callback); efi::Status::SUCCESS } @@ -362,7 +362,7 @@ mod test { boot_services.expect_open_protocol().returning(|handle, protocol, interface, agent, controller, attributes| { assert_eq!(handle, 0x1234 as efi::Handle); assert_eq!(unsafe { *protocol }, hid_io::protocol::GUID); - assert_ne!(interface, core::ptr::null_mut()); + assert_ne!(interface, ptr::null_mut()); assert_eq!(agent, 0x4321 as efi::Handle); assert_eq!(controller, 0x1234 as efi::Handle); assert_eq!(attributes, efi::OPEN_PROTOCOL_BY_DRIVER); diff --git a/HidPkg/UefiHidDxeV2/src/keyboard/mod.rs b/HidPkg/UefiHidDxeV2/src/keyboard.rs similarity index 93% rename from HidPkg/UefiHidDxeV2/src/keyboard/mod.rs rename to HidPkg/UefiHidDxeV2/src/keyboard.rs index 0fea4402e4..5f7aedd8e5 100644 --- a/HidPkg/UefiHidDxeV2/src/keyboard/mod.rs +++ b/HidPkg/UefiHidDxeV2/src/keyboard.rs @@ -13,19 +13,21 @@ mod key_queue; mod simple_text_in; mod simple_text_in_ex; -use core::ffi::c_void; - use alloc::{ boxed::Box, collections::{BTreeMap, BTreeSet}, vec, vec::Vec, }; +use core::{ffi::c_void, ptr}; + +use r_efi::{efi, hii, protocols}; + use hidparser::{ report_data_types::{ReportId, Usage}, ArrayField, ReportDescriptor, ReportField, VariableField, }; -use r_efi::{efi, hii, protocols}; +use rust_advanced_logger_dxe::{debugln, function, DEBUG_ERROR, DEBUG_VERBOSE, DEBUG_WARN}; use crate::{ boot_services::UefiBootServices, @@ -33,8 +35,6 @@ use crate::{ keyboard::key_queue::OrdKeyData, }; -use rust_advanced_logger_dxe::{debugln, function, DEBUG_ERROR, DEBUG_VERBOSE, DEBUG_WARN}; - // usages supported by this module const KEYBOARD_MODIFIER_USAGE_MIN: u32 = 0x000700E0; const KEYBOARD_MODIFIER_USAGE_MAX: u32 = 0x000700E7; @@ -182,10 +182,7 @@ impl KeyboardHidHandler { Err(efi::Status::DEVICE_ERROR)?; } - report_builder.report_size = report.size_in_bits / 8; - if (report.size_in_bits % 8) != 0 { - report_builder.report_size += 1; - } + report_builder.report_size = usize::div_ceil(report.size_in_bits, 8); for field in &report.fields { match field { @@ -219,10 +216,8 @@ impl KeyboardHidHandler { // Helper routine to handle variable keyboard input report fields fn handle_variable_key(&mut self, field: VariableField, report: &[u8]) { match field.field_value(report) { - Some(x) if x != 0 => { - self.current_keys.insert(field.usage); - } - None | Some(_) => (), + Some(x) if x != 0 => _ = self.current_keys.insert(field.usage), + _ => (), } } @@ -244,7 +239,7 @@ impl KeyboardHidHandler { self.current_keys.insert(Usage::from(usage)); } } - None | Some(_) => (), + _ => (), } } @@ -262,7 +257,7 @@ impl KeyboardHidHandler { // LEDs. fn generate_led_output_reports(&mut self) -> Vec<(Option, Vec)> { let mut output_vec = Vec::new(); - let current_leds: BTreeSet = self.key_queue.get_active_leds().iter().cloned().collect(); + let current_leds: BTreeSet = self.key_queue.active_leds().iter().cloned().collect(); if current_leds != self.led_state { self.led_state = current_leds; for output_builder in self.output_builders.clone() { @@ -292,14 +287,14 @@ impl KeyboardHidHandler { let context = LayoutChangeContext { boot_services: self.boot_services, keyboard_handler: self as *mut Self }; let context_ptr = Box::into_raw(Box::new(context)); - let mut layout_change_event: efi::Event = core::ptr::null_mut(); + let mut layout_change_event: efi::Event = ptr::null_mut(); let status = self.boot_services.create_event_ex( efi::EVT_NOTIFY_SIGNAL, efi::TPL_NOTIFY, Some(on_layout_update), context_ptr as *mut c_void, &protocols::hii_database::SET_KEYBOARD_LAYOUT_EVENT_GUID, - core::ptr::addr_of_mut!(layout_change_event), + ptr::addr_of_mut!(layout_change_event), ); if status.is_error() { Err(status)?; @@ -322,26 +317,26 @@ impl KeyboardHidHandler { //instance. Leaking context allows context usage in the callback should it fire. debugln!(DEBUG_ERROR, "Failed to close layout_change_event event, status: {:x?}", status); unsafe { - (*self.layout_context).keyboard_handler = core::ptr::null_mut(); + (*self.layout_context).keyboard_handler = ptr::null_mut(); } return Err(status); } // safe to drop layout change context. drop(unsafe { Box::from_raw(self.layout_context) }); - self.layout_context = core::ptr::null_mut(); - self.layout_change_event = core::ptr::null_mut(); + self.layout_context = ptr::null_mut(); + self.layout_change_event = ptr::null_mut(); } Ok(()) } // Installs a default keyboard layout. fn install_default_layout(&mut self) -> Result<(), efi::Status> { - let mut hii_database_protocol_ptr: *mut protocols::hii_database::Protocol = core::ptr::null_mut(); + let mut hii_database_protocol_ptr: *mut protocols::hii_database::Protocol = ptr::null_mut(); let status = self.boot_services.locate_protocol( &protocols::hii_database::PROTOCOL_GUID as *const efi::Guid as *mut efi::Guid, - core::ptr::null_mut(), - core::ptr::addr_of_mut!(hii_database_protocol_ptr) as *mut *mut c_void, + ptr::null_mut(), + ptr::addr_of_mut!(hii_database_protocol_ptr) as *mut *mut c_void, ); if status.is_error() { debugln!( @@ -356,12 +351,12 @@ impl KeyboardHidHandler { hii_database_protocol_ptr.as_mut().expect("Bad pointer returned from successful locate protocol.") }; - let mut hii_handle: hii::Handle = core::ptr::null_mut(); + let mut hii_handle: hii::Handle = ptr::null_mut(); let status = (hii_database_protocol.new_package_list)( hii_database_protocol_ptr, hii_keyboard_layout::get_default_keyboard_pkg_list_buffer().as_ptr() as *const hii::PackageListHeader, - core::ptr::null_mut(), - core::ptr::addr_of_mut!(hii_handle), + ptr::null_mut(), + ptr::addr_of_mut!(hii_handle), ); if status.is_error() { @@ -377,7 +372,6 @@ impl KeyboardHidHandler { hii_database_protocol_ptr, &hii_keyboard_layout::DEFAULT_KEYBOARD_LAYOUT_GUID as *const efi::Guid as *mut efi::Guid, ); - if status.is_error() { debugln!(DEBUG_ERROR, "keyboard::install_default_layout: Failed to set keyboard layout: {:x?}", status); Err(status)?; @@ -394,7 +388,7 @@ impl KeyboardHidHandler { on_layout_update(self.layout_change_event, self.layout_context as *mut c_void); //install a default layout if no layout is installed. - if self.key_queue.get_layout().is_none() { + if self.key_queue.layout().is_none() { self.install_default_layout()?; } Ok(()) @@ -483,7 +477,7 @@ impl KeyboardHidHandler { /// Returns the set of keys that have pending callbacks, along with the vector of callback functions associated with /// each key. - pub fn get_pending_callbacks( + pub fn pending_callbacks( &mut self, ) -> (Option, Vec) { @@ -501,12 +495,12 @@ impl KeyboardHidHandler { } /// Returns the agent associated with this KeyboardHidHandler - pub fn get_agent(&self) -> efi::Handle { + pub fn agent(&self) -> efi::Handle { self.agent } /// Returns the controller associated with this KeyboardHidHandler. - pub fn get_controller(&self) -> Option { + pub fn controller(&self) -> Option { self.controller } @@ -529,11 +523,8 @@ impl HidReportReceiver for KeyboardHidHandler { fn initialize(&mut self, controller: efi::Handle, hid_io: &dyn HidIo) -> Result<(), efi::Status> { let descriptor = hid_io.get_report_descriptor()?; self.process_descriptor(descriptor)?; - self.install_protocol_interfaces(controller)?; - self.initialize_keyboard_layout()?; - Ok(()) } @@ -568,7 +559,6 @@ impl HidReportReceiver for KeyboardHidHandler { report.len() ); debugln!(DEBUG_VERBOSE, "report: {:x?}", report); - //break 'report_processing; } //reset currently active keys to empty set. @@ -632,7 +622,6 @@ impl HidReportReceiver for KeyboardHidHandler { let result = hid_io.set_output_report(id.map(|x| u32::from(x) as u8), &output_report); if let Err(result) = result { debugln!(DEBUG_ERROR, "unexpected error sending output report: {:?}", result); - let _ = result; } } } @@ -641,17 +630,17 @@ impl HidReportReceiver for KeyboardHidHandler { impl Drop for KeyboardHidHandler { fn drop(&mut self) { if let Some(controller) = self.controller { - let status = simple_text_in::SimpleTextInFfi::uninstall(self.boot_services, self.agent, controller); - if status.is_err() { + if let Err(status) = simple_text_in::SimpleTextInFfi::uninstall(self.boot_services, self.agent, controller) + { debugln!(DEBUG_ERROR, "KeyboardHidHandler::drop: Failed to uninstall simple_text_in: {:?}", status); } - let status = simple_text_in_ex::SimpleTextInExFfi::uninstall(self.boot_services, self.agent, controller); - if status.is_err() { + if let Err(status) = + simple_text_in_ex::SimpleTextInExFfi::uninstall(self.boot_services, self.agent, controller) + { debugln!(DEBUG_ERROR, "KeyboardHidHandler::drop: Failed to uninstall simple_text_in: {:?}", status); } } - let status = self.uninstall_layout_change_event(); - if status.is_err() { + if let Err(status) = self.uninstall_layout_change_event() { debugln!(DEBUG_ERROR, "KeyboardHidHandler::drop: Failed to close layout_change_event: {:?}", status); } } @@ -670,11 +659,11 @@ extern "efiapi" fn on_layout_update(_event: efi::Event, context: *mut c_void) { let keyboard_handler = unsafe { context.keyboard_handler.as_mut() }.expect("bad keyboard handler"); - let mut hii_database_protocol_ptr: *mut protocols::hii_database::Protocol = core::ptr::null_mut(); + let mut hii_database_protocol_ptr: *mut protocols::hii_database::Protocol = ptr::null_mut(); let status = context.boot_services.locate_protocol( &protocols::hii_database::PROTOCOL_GUID as *const efi::Guid as *mut efi::Guid, - core::ptr::null_mut(), - core::ptr::addr_of_mut!(hii_database_protocol_ptr) as *mut *mut c_void, + ptr::null_mut(), + ptr::addr_of_mut!(hii_database_protocol_ptr) as *mut *mut c_void, ); if status.is_error() { @@ -690,9 +679,9 @@ extern "efiapi" fn on_layout_update(_event: efi::Event, context: *mut c_void) { let mut layout_buffer_len: u16 = 0; match (hii_database_protocol.get_keyboard_layout)( hii_database_protocol_ptr, - core::ptr::null_mut(), + ptr::null_mut(), &mut layout_buffer_len as *mut u16, - core::ptr::null_mut(), + ptr::null_mut(), ) { efi::Status::NOT_FOUND => break 'layout_processing, status if status != efi::Status::BUFFER_TOO_SMALL => { @@ -710,7 +699,7 @@ extern "efiapi" fn on_layout_update(_event: efi::Event, context: *mut c_void) { let mut keyboard_layout_buffer = vec![0u8; layout_buffer_len as usize]; let status = (hii_database_protocol.get_keyboard_layout)( hii_database_protocol_ptr, - core::ptr::null_mut(), + ptr::null_mut(), &mut layout_buffer_len as *mut u16, keyboard_layout_buffer.as_mut_ptr() as *mut protocols::hii_database::KeyboardLayout<0>, ); @@ -1213,8 +1202,8 @@ mod test { | protocols::simple_text_input_ex::CAPS_LOCK_ACTIVE ); - assert_eq!(keyboard_handler.get_controller(), keyboard_handler.controller); - assert_eq!(keyboard_handler.get_agent(), keyboard_handler.agent); + assert_eq!(keyboard_handler.controller(), keyboard_handler.controller); + assert_eq!(keyboard_handler.agent(), keyboard_handler.agent); } #[test] @@ -1295,7 +1284,7 @@ mod test { key_data.key_state.key_shift_state = protocols::simple_text_input_ex::SHIFT_STATE_VALID; key_data.key_state.key_toggle_state = protocols::simple_text_input_ex::TOGGLE_STATE_VALID; - let (callback_key_data, callbacks) = keyboard_handler.get_pending_callbacks(); + let (callback_key_data, callbacks) = keyboard_handler.pending_callbacks(); assert_eq!(OrdKeyData(key_data), OrdKeyData(callback_key_data.unwrap())); assert!(callbacks.contains( &(mock_key_notify_callback @@ -1306,7 +1295,7 @@ mod test { as extern "efiapi" fn(*mut protocols::simple_text_input_ex::KeyData) -> efi::Status) )); - let (callback_key_data, callbacks) = keyboard_handler.get_pending_callbacks(); + let (callback_key_data, callbacks) = keyboard_handler.pending_callbacks(); assert!(callback_key_data.is_none()); assert!(callbacks.is_empty()); @@ -1320,7 +1309,7 @@ mod test { keyboard_handler.receive_report(report, &hid_io); loop { - let (callback_key_data, callbacks) = keyboard_handler.get_pending_callbacks(); + let (callback_key_data, callbacks) = keyboard_handler.pending_callbacks(); if let Some(callback_key_data) = callback_key_data { match callback_key_data.key.unicode_char { char if char == 'a' as u16 || char == 'c' as u16 => { @@ -1347,7 +1336,7 @@ mod test { let report: &[u8] = &[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; keyboard_handler.receive_report(report, &hid_io); - let (callback_key_data, callbacks) = keyboard_handler.get_pending_callbacks(); + let (callback_key_data, callbacks) = keyboard_handler.pending_callbacks(); assert!(callback_key_data.is_none()); assert!(callbacks.is_empty()); } diff --git a/HidPkg/UefiHidDxeV2/src/keyboard/key_queue.rs b/HidPkg/UefiHidDxeV2/src/keyboard/key_queue.rs index f7fcd9b068..bee0bcf55f 100644 --- a/HidPkg/UefiHidDxeV2/src/keyboard/key_queue.rs +++ b/HidPkg/UefiHidDxeV2/src/keyboard/key_queue.rs @@ -9,19 +9,17 @@ //! SPDX-License-Identifier: BSD-2-Clause-Patent //! -use core::sync::atomic::Ordering; - use alloc::{ collections::{BTreeSet, VecDeque}, vec::Vec, }; +use core::{ops::Deref, sync::atomic::Ordering}; use hidparser::report_data_types::Usage; -use hii_keyboard_layout::{EfiKey, HiiKey, HiiKeyDescriptor, HiiKeyboardLayout, HiiNsKeyDescriptor}; +use hii_keyboard_layout::{EfiKey, HiiKey, HiiKeyboardLayout, HiiNsKeyDescriptor}; use r_efi::{ efi, protocols::{self, hii_database::*, simple_text_input::InputKey, simple_text_input_ex::*}, }; - use rust_advanced_logger_dxe::{debugln, DEBUG_WARN}; use crate::RUNTIME_SERVICES; @@ -31,7 +29,8 @@ use crate::RUNTIME_SERVICES; const KEYBOARD_MODIFIERS: &[u16] = &[ LEFT_CONTROL_MODIFIER, RIGHT_CONTROL_MODIFIER, LEFT_SHIFT_MODIFIER, RIGHT_SHIFT_MODIFIER, LEFT_ALT_MODIFIER, RIGHT_ALT_MODIFIER, LEFT_LOGO_MODIFIER, RIGHT_LOGO_MODIFIER, MENU_MODIFIER, PRINT_MODIFIER, SYS_REQUEST_MODIFIER, - ALT_GR_MODIFIER]; + ALT_GR_MODIFIER, +]; // The set of HID usages that represent modifier keys that toggle state (as opposed to remain active while pressed). const TOGGLE_MODIFIERS: &[u16] = &[NUM_LOCK_MODIFIER, CAPS_LOCK_MODIFIER, SCROLL_LOCK_MODIFIER]; @@ -41,34 +40,42 @@ const CTRL_MODIFIERS: &[u16] = &[LEFT_CONTROL_MODIFIER, RIGHT_CONTROL_MODIFIER]; const SHIFT_MODIFIERS: &[u16] = &[LEFT_SHIFT_MODIFIER, RIGHT_SHIFT_MODIFIER]; const ALT_MODIFIERS: &[u16] = &[LEFT_ALT_MODIFIER, RIGHT_ALT_MODIFIER]; -// Defines whether a key stroke represents a key being pressed (KeyDown) or released (KeyUp) +/// Defines whether a key stroke represents a key being pressed (KeyDown) or released (KeyUp) #[derive(Debug, PartialEq, Eq)] pub(crate) enum KeyAction { - // Key is being pressed - KeyUp, - // Key is being released + /// Key is being pressed KeyDown, + /// Key is being released + KeyUp, } -// A wrapper for the KeyData type that allows definition of the Ord trait and additional registration matching logic. +/// A wrapper for the KeyData type that allows definition of the Ord trait and additional registration matching logic. #[derive(Debug, Clone)] pub(crate) struct OrdKeyData(pub protocols::simple_text_input_ex::KeyData); +impl Deref for OrdKeyData { + type Target = protocols::simple_text_input_ex::KeyData; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + impl Ord for OrdKeyData { fn cmp(&self, other: &Self) -> core::cmp::Ordering { - let e = self.0.key.unicode_char.cmp(&other.0.key.unicode_char); + let e = self.key.unicode_char.cmp(&other.key.unicode_char); if !e.is_eq() { return e; } - let e = self.0.key.scan_code.cmp(&other.0.key.scan_code); + let e = self.key.scan_code.cmp(&other.key.scan_code); if !e.is_eq() { return e; } - let e = self.0.key_state.key_shift_state.cmp(&other.0.key_state.key_shift_state); + let e = self.key_state.key_shift_state.cmp(&other.key_state.key_shift_state); if !e.is_eq() { return e; } - self.0.key_state.key_toggle_state.cmp(&other.0.key_state.key_toggle_state) + self.key_state.key_toggle_state.cmp(&other.key_state.key_toggle_state) } } @@ -91,14 +98,14 @@ impl OrdKeyData { // allows for some degree of wildcard matching. Refer to UEFI spec 2.10 section 12.2.5. pub(crate) fn matches_registered_key(&self, registration: &Self) -> bool { // assign names here for brevity below. - let self_char = self.0.key.unicode_char; - let self_scan = self.0.key.scan_code; - let self_shift = self.0.key_state.key_shift_state; - let self_toggle = self.0.key_state.key_toggle_state; - let register_char = registration.0.key.unicode_char; - let register_scan = registration.0.key.scan_code; - let register_shift = registration.0.key_state.key_shift_state; - let register_toggle = registration.0.key_state.key_toggle_state; + let self_char = self.key.unicode_char; + let self_scan = self.key.scan_code; + let self_shift = self.key_state.key_shift_state; + let self_toggle = self.key_state.key_toggle_state; + let register_char = registration.key.unicode_char; + let register_scan = registration.key.scan_code; + let register_shift = registration.key_state.key_shift_state; + let register_toggle = registration.key_state.key_toggle_state; //char and scan must match (per the reference implementation in the EDK2 C code). if !(register_char == self_char && register_scan == self_scan) { @@ -136,7 +143,7 @@ impl KeyQueue { if extended_reset { self.active_modifiers.clear(); } else { - let active_leds = self.get_active_led_modifiers(); + let active_leds = self.active_led_modifiers(); self.active_modifiers.retain(|x| active_leds.contains(x)); } self.active_ns_key = None; @@ -159,7 +166,7 @@ impl KeyQueue { // Check if it is a dependent key of a currently active "non-spacing" (ns) key. // Non-spacing key handling is described in UEFI spec 2.10 section 33.2.4.3. - let mut current_descriptor: Option = None; + let mut current_descriptor = None; if let Some(ref ns_key) = self.active_ns_key { for descriptor in &ns_key.dependent_keys { if descriptor.key == efi_key { @@ -176,38 +183,33 @@ impl KeyQueue { if current_descriptor.is_none() { for key in &active_layout.keys { match key { - HiiKey::Key(descriptor) => { - if descriptor.key == efi_key { - current_descriptor = Some(*descriptor); - break; - } + HiiKey::Key(descriptor) if descriptor.key == efi_key => { + current_descriptor = Some(*descriptor); + break; } - HiiKey::NsKey(ns_descriptor) => { - if ns_descriptor.descriptor.key == efi_key { - // if it is an ns_key, set it as the active ns key, and no further processing is needed. - self.active_ns_key = Some(ns_descriptor.clone()); - return; - } + HiiKey::NsKey(ns_descriptor) if ns_descriptor.descriptor.key == efi_key => { + // if it is an ns_key, set it as the active ns key, and no further processing is needed. + self.active_ns_key = Some(ns_descriptor.clone()); + return; } + _ => continue, } } } - if current_descriptor.is_none() { + let Some(current_descriptor) = current_descriptor else { return; //could not find descriptor, nothing to do. - } - - let current_descriptor = current_descriptor.unwrap(); + }; //handle modifiers that are active as long as they are pressed if KEYBOARD_MODIFIERS.contains(¤t_descriptor.modifier) { match action { - KeyAction::KeyUp => { - self.active_modifiers.remove(¤t_descriptor.modifier); - } KeyAction::KeyDown => { self.active_modifiers.insert(current_descriptor.modifier); } + KeyAction::KeyUp => { + self.active_modifiers.remove(¤t_descriptor.modifier); + } } } @@ -401,16 +403,16 @@ impl KeyQueue { self.partial_key_support_active = (toggle_state & KEY_STATE_EXPOSED) != 0; } - fn get_active_led_modifiers(&self) -> Vec { + fn active_led_modifiers(&self) -> Vec { self.active_modifiers.iter().cloned().filter(|x| modifier_to_led_usage(*x).is_some()).collect() } // Returns a vector of HID usages corresponding to the active LEDs based on the active modifier state. - pub(crate) fn get_active_leds(&self) -> Vec { + pub(crate) fn active_leds(&self) -> Vec { self.active_modifiers.iter().cloned().filter_map(modifier_to_led_usage).collect() } // Returns the current keyboard layout that the KeyQueue is using. - pub(crate) fn get_layout(&self) -> Option { + pub(crate) fn layout(&self) -> Option { self.layout.clone() } diff --git a/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in.rs b/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in.rs index 2b8c7e9170..94ce4acc7d 100644 --- a/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in.rs +++ b/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in.rs @@ -9,7 +9,7 @@ //! use alloc::boxed::Box; -use core::ffi::c_void; +use core::{ffi::c_void, ptr}; use r_efi::{efi, protocols}; use rust_advanced_logger_dxe::{debugln, DEBUG_ERROR}; @@ -51,7 +51,7 @@ impl SimpleTextInFfi { simple_text_in: protocols::simple_text_input::Protocol { reset: Self::simple_text_in_reset, read_key_stroke: Self::simple_text_in_read_key_stroke, - wait_for_key: core::ptr::null_mut(), + wait_for_key: ptr::null_mut(), }, boot_services, keyboard_handler: keyboard_handler as *mut KeyboardHidHandler, @@ -60,13 +60,13 @@ impl SimpleTextInFfi { let simple_text_in_ptr = Box::into_raw(Box::new(simple_text_in_ctx)); //create event for wait_for_key - let mut wait_for_key_event: efi::Event = core::ptr::null_mut(); + let mut wait_for_key_event: efi::Event = ptr::null_mut(); let status = boot_services.create_event( efi::EVT_NOTIFY_WAIT, efi::TPL_NOTIFY, Some(Self::simple_text_in_wait_for_key), simple_text_in_ptr as *mut c_void, - core::ptr::addr_of_mut!(wait_for_key_event), + ptr::addr_of_mut!(wait_for_key_event), ); if status.is_error() { drop(unsafe { Box::from_raw(simple_text_in_ptr) }); @@ -78,7 +78,7 @@ impl SimpleTextInFfi { //install the simple_text_in protocol let mut controller = controller; let status = boot_services.install_protocol_interface( - core::ptr::addr_of_mut!(controller), + ptr::addr_of_mut!(controller), &protocols::simple_text_input::PROTOCOL_GUID as *const efi::Guid as *mut efi::Guid, efi::NATIVE_INTERFACE, simple_text_in_ptr as *mut c_void, @@ -101,11 +101,11 @@ impl SimpleTextInFfi { ) -> Result<(), efi::Status> { //Controller is set - that means initialize() was called, and there is potential state exposed thru FFI that needs //to be cleaned up. - let mut simple_text_in_ptr: *mut SimpleTextInFfi = core::ptr::null_mut(); + let mut simple_text_in_ptr: *mut SimpleTextInFfi = ptr::null_mut(); let status = boot_services.open_protocol( controller, &protocols::simple_text_input::PROTOCOL_GUID as *const efi::Guid as *mut efi::Guid, - core::ptr::addr_of_mut!(simple_text_in_ptr) as *mut *mut c_void, + ptr::addr_of_mut!(simple_text_in_ptr) as *mut *mut c_void, agent, controller, efi::OPEN_PROTOCOL_GET_PROTOCOL, @@ -130,7 +130,7 @@ impl SimpleTextInFfi { debugln!(DEBUG_ERROR, "Failed to uninstall simple_text_in interface, status: {:x?}", status); unsafe { - (*simple_text_in_ptr).keyboard_handler = core::ptr::null_mut(); + (*simple_text_in_ptr).keyboard_handler = ptr::null_mut(); } //return without tearing down the context. return Err(status); @@ -146,7 +146,7 @@ impl SimpleTextInFfi { //and return error based on observing keyboard_handler is null. debugln!(DEBUG_ERROR, "Failed to close simple_text_in_ptr.wait_for_key event, status: {:x?}", status); unsafe { - (*simple_text_in_ptr).keyboard_handler = core::ptr::null_mut(); + (*simple_text_in_ptr).keyboard_handler = ptr::null_mut(); } return Err(status); } @@ -170,8 +170,8 @@ impl SimpleTextInFfi { let keyboard_handler = unsafe { context.keyboard_handler.as_mut() }; if let Some(keyboard_handler) = keyboard_handler { //reset requires an instance of hid_io to allow for LED updating, so use UefiHidIoFactory to build one. - if let Some(controller) = keyboard_handler.get_controller() { - let hid_io = UefiHidIoFactory::new(context.boot_services, keyboard_handler.get_agent()) + if let Some(controller) = keyboard_handler.controller() { + let hid_io = UefiHidIoFactory::new(context.boot_services, keyboard_handler.agent()) .new_hid_io(controller, false); if let Ok(hid_io) = hid_io { if let Err(err) = keyboard_handler.reset(hid_io.as_ref(), extended_verification.into()) { @@ -268,7 +268,8 @@ mod test { use core::{ ffi::c_void, mem::MaybeUninit, - sync::atomic::{AtomicBool, AtomicPtr}, + ptr, + sync::atomic::{AtomicBool, AtomicPtr, Ordering}, }; use crate::{ @@ -339,13 +340,13 @@ mod test { #[test] fn uninstall_should_uninstall_simple_text_in_interface() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); let boot_services = create_fake_static_boot_service(); // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, interface| { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); efi::Status::SUCCESS }); @@ -353,14 +354,14 @@ mod test { boot_services.expect_open_protocol().returning(|_, protocol, interface, _, _, _| { unsafe { assert_eq!(protocol.read(), protocols::simple_text_input::PROTOCOL_GUID); - interface.write(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst)); + interface.write(CONTEXT_PTR.load(Ordering::SeqCst)); } efi::Status::SUCCESS }); boot_services.expect_uninstall_protocol_interface().returning(|_, protocol, interface| { unsafe { assert_eq!(protocol.read(), protocols::simple_text_input::PROTOCOL_GUID); - assert_eq!(interface, CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst)); + assert_eq!(interface, CONTEXT_PTR.load(Ordering::SeqCst)); } efi::Status::SUCCESS }); @@ -369,20 +370,20 @@ mod test { let mut keyboard_handler = KeyboardHidHandler::new(boot_services, 1 as efi::Handle); SimpleTextInFfi::install(boot_services, 2 as efi::Handle, &mut keyboard_handler).unwrap(); - assert_ne!(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), core::ptr::null_mut()); + assert_ne!(CONTEXT_PTR.load(Ordering::SeqCst), ptr::null_mut()); SimpleTextInFfi::uninstall(boot_services, 1 as efi::Handle, 2 as efi::Handle).unwrap(); } #[test] fn reset_should_invoke_reset() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); let boot_services = create_fake_static_boot_service(); // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, interface| { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); efi::Status::SUCCESS }); @@ -422,7 +423,7 @@ mod test { boot_services.expect_uninstall_protocol_interface().returning(|_, protocol, interface| { unsafe { assert_eq!(protocol.read(), protocols::simple_text_input::PROTOCOL_GUID); - assert_eq!(interface, CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst)); + assert_eq!(interface, CONTEXT_PTR.load(Ordering::SeqCst)); } efi::Status::SUCCESS }); @@ -432,10 +433,9 @@ mod test { keyboard_handler.set_controller(Some(2 as efi::Handle)); SimpleTextInFfi::install(boot_services, 2 as efi::Handle, &mut keyboard_handler).unwrap(); - assert_ne!(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), core::ptr::null_mut()); + assert_ne!(CONTEXT_PTR.load(Ordering::SeqCst), ptr::null_mut()); - let this = - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst) as *mut protocols::simple_text_input::Protocol; + let this = CONTEXT_PTR.load(Ordering::SeqCst) as *mut protocols::simple_text_input::Protocol; SimpleTextInFfi::simple_text_in_reset(this, efi::Boolean::FALSE); // avoid keyboard drop uninstall flows @@ -444,13 +444,13 @@ mod test { #[test] fn read_key_stroke_should_read_keystrokes() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); let boot_services = create_fake_static_boot_service(); // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, interface| { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); efi::Status::SUCCESS }); @@ -475,7 +475,7 @@ mod test { keyboard_handler.initialize(2 as efi::Handle, &hid_io).unwrap(); SimpleTextInFfi::install(boot_services, 2 as efi::Handle, &mut keyboard_handler).unwrap(); - assert_ne!(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), core::ptr::null_mut()); + assert_ne!(CONTEXT_PTR.load(Ordering::SeqCst), ptr::null_mut()); let report: &[u8] = &[0x00, 0x00, 0x04, 0x05, 0x06, 0x00, 0x00, 0x00]; keyboard_handler.receive_report(report, &hid_io); @@ -484,8 +484,7 @@ mod test { keyboard_handler.receive_report(report, &hid_io); //read with simple_text_in - let this = - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst) as *mut protocols::simple_text_input::Protocol; + let this = CONTEXT_PTR.load(Ordering::SeqCst) as *mut protocols::simple_text_input::Protocol; let mut input_key: protocols::simple_text_input::InputKey = Default::default(); let status = SimpleTextInFfi::simple_text_in_read_key_stroke( @@ -582,7 +581,7 @@ mod test { #[test] fn wait_for_key_should_wait_for_key() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); static RECEIVED_EVENT: AtomicBool = AtomicBool::new(false); let boot_services = create_fake_static_boot_service(); @@ -592,7 +591,7 @@ mod test { boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); boot_services.expect_install_protocol_interface().returning(|_, protocol, _, interface| { if unsafe { *protocol } == protocols::simple_text_input::PROTOCOL_GUID { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); } efi::Status::SUCCESS }); @@ -601,7 +600,7 @@ mod test { boot_services.expect_restore_tpl().returning(|_| ()); boot_services.expect_signal_event().returning(|_| { - RECEIVED_EVENT.store(true, core::sync::atomic::Ordering::SeqCst); + RECEIVED_EVENT.store(true, Ordering::SeqCst); efi::Status::SUCCESS }); @@ -616,12 +615,9 @@ mod test { keyboard_handler.set_controller(Some(2 as efi::Handle)); keyboard_handler.initialize(2 as efi::Handle, &hid_io).unwrap(); - RECEIVED_EVENT.store(false, core::sync::atomic::Ordering::SeqCst); - SimpleTextInFfi::simple_text_in_wait_for_key( - 3 as efi::Event, - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), - ); - assert!(!RECEIVED_EVENT.load(core::sync::atomic::Ordering::SeqCst)); + RECEIVED_EVENT.store(false, Ordering::SeqCst); + SimpleTextInFfi::simple_text_in_wait_for_key(3 as efi::Event, CONTEXT_PTR.load(Ordering::SeqCst)); + assert!(!RECEIVED_EVENT.load(Ordering::SeqCst)); // press the a key let report: &[u8] = &[0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00]; @@ -631,11 +627,8 @@ mod test { let report: &[u8] = &[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; keyboard_handler.receive_report(report, &hid_io); - SimpleTextInFfi::simple_text_in_wait_for_key( - 3 as efi::Event, - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), - ); - assert!(RECEIVED_EVENT.load(core::sync::atomic::Ordering::SeqCst)); + SimpleTextInFfi::simple_text_in_wait_for_key(3 as efi::Event, CONTEXT_PTR.load(Ordering::SeqCst)); + assert!(RECEIVED_EVENT.load(Ordering::SeqCst)); // avoid keyboard drop uninstall flows keyboard_handler.set_controller(None); diff --git a/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in_ex.rs b/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in_ex.rs index ea71486418..62ecdfe300 100644 --- a/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in_ex.rs +++ b/HidPkg/UefiHidDxeV2/src/keyboard/simple_text_in_ex.rs @@ -8,7 +8,7 @@ //! SPDX-License-Identifier: BSD-2-Clause-Patent //! use alloc::{boxed::Box, vec::Vec}; -use core::ffi::c_void; +use core::{ffi::c_void, ptr}; use r_efi::{efi, protocols}; use rust_advanced_logger_dxe::{debugln, DEBUG_ERROR}; @@ -51,26 +51,26 @@ impl SimpleTextInExFfi { simple_text_in_ex: protocols::simple_text_input_ex::Protocol { reset: Self::simple_text_in_ex_reset, read_key_stroke_ex: Self::simple_text_in_ex_read_key_stroke, - wait_for_key_ex: core::ptr::null_mut(), + wait_for_key_ex: ptr::null_mut(), set_state: Self::simple_text_in_ex_set_state, register_key_notify: Self::simple_text_in_ex_register_key_notify, unregister_key_notify: Self::simple_text_in_ex_unregister_key_notify, }, boot_services, - key_notify_event: core::ptr::null_mut(), + key_notify_event: ptr::null_mut(), keyboard_handler: keyboard_handler as *mut KeyboardHidHandler, }; let simple_text_in_ex_ptr = Box::into_raw(Box::new(simple_text_in_ex_ctx)); //create event for wait_for_key - let mut wait_for_key_event: efi::Event = core::ptr::null_mut(); + let mut wait_for_key_event: efi::Event = ptr::null_mut(); let status = boot_services.create_event( efi::EVT_NOTIFY_WAIT, efi::TPL_NOTIFY, Some(Self::simple_text_in_ex_wait_for_key), simple_text_in_ex_ptr as *mut c_void, - core::ptr::addr_of_mut!(wait_for_key_event), + ptr::addr_of_mut!(wait_for_key_event), ); if status.is_error() { drop(unsafe { Box::from_raw(simple_text_in_ex_ptr) }); @@ -82,13 +82,13 @@ impl SimpleTextInExFfi { //Key notifies are required to dispatch at TPL_CALLBACK per UEFI spec 2.10 section 12.2.5. The keyboard handler //interfaces run at TPL_NOTIFY and issue a boot_services.signal_event() on this event to pend key notifies to be //serviced at TPL_CALLBACK. - let mut key_notify_event: efi::Event = core::ptr::null_mut(); + let mut key_notify_event: efi::Event = ptr::null_mut(); let status = boot_services.create_event( efi::EVT_NOTIFY_SIGNAL, efi::TPL_CALLBACK, Some(Self::process_key_notifies), simple_text_in_ex_ptr as *mut c_void, - core::ptr::addr_of_mut!(key_notify_event), + ptr::addr_of_mut!(key_notify_event), ); if status.is_error() { let _ = boot_services.close_event(wait_for_key_event); @@ -100,7 +100,7 @@ impl SimpleTextInExFfi { //install the simple_text_in_ex protocol let mut controller = controller; let status = boot_services.install_protocol_interface( - core::ptr::addr_of_mut!(controller), + ptr::addr_of_mut!(controller), &protocols::simple_text_input_ex::PROTOCOL_GUID as *const efi::Guid as *mut efi::Guid, efi::NATIVE_INTERFACE, simple_text_in_ex_ptr as *mut c_void, @@ -124,11 +124,11 @@ impl SimpleTextInExFfi { ) -> Result<(), efi::Status> { //Controller is set - that means initialize() was called, and there is potential state exposed thru FFI that needs //to be cleaned up. - let mut simple_text_in_ex_ptr: *mut SimpleTextInExFfi = core::ptr::null_mut(); + let mut simple_text_in_ex_ptr: *mut SimpleTextInExFfi = ptr::null_mut(); let status = boot_services.open_protocol( controller, &protocols::simple_text_input_ex::PROTOCOL_GUID as *const efi::Guid as *mut efi::Guid, - core::ptr::addr_of_mut!(simple_text_in_ex_ptr) as *mut *mut c_void, + ptr::addr_of_mut!(simple_text_in_ex_ptr) as *mut *mut c_void, agent, controller, efi::OPEN_PROTOCOL_GET_PROTOCOL, @@ -153,7 +153,7 @@ impl SimpleTextInExFfi { debugln!(DEBUG_ERROR, "Failed to uninstall simple_text_in interface, status: {:x?}", status); unsafe { - (*simple_text_in_ex_ptr).keyboard_handler = core::ptr::null_mut(); + (*simple_text_in_ex_ptr).keyboard_handler = ptr::null_mut(); } //return without tearing down the context. return Err(status); @@ -169,7 +169,7 @@ impl SimpleTextInExFfi { //and return error based on observing keyboard_handler is null. debugln!(DEBUG_ERROR, "Failed to close simple_text_in_ptr.wait_for_key event, status: {:x?}", status); unsafe { - (*simple_text_in_ex_ptr).keyboard_handler = core::ptr::null_mut(); + (*simple_text_in_ex_ptr).keyboard_handler = ptr::null_mut(); } return Err(status); } @@ -184,7 +184,7 @@ impl SimpleTextInExFfi { //and return error based on observing keyboard_handler is null. debugln!(DEBUG_ERROR, "Failed to close key_notify_event event, status: {:x?}", status); unsafe { - (*simple_text_in_ex_ptr).keyboard_handler = core::ptr::null_mut(); + (*simple_text_in_ex_ptr).keyboard_handler = ptr::null_mut(); } return Err(status); } @@ -204,24 +204,19 @@ impl SimpleTextInExFfi { } let context = unsafe { (this as *mut SimpleTextInExFfi).as_mut() }.expect("bad pointer"); let old_tpl = context.boot_services.raise_tpl(efi::TPL_NOTIFY); - let mut status = efi::Status::DEVICE_ERROR; - '_reset_processing: { - let keyboard_handler = unsafe { context.keyboard_handler.as_mut() }; - if let Some(keyboard_handler) = keyboard_handler { - //reset requires an instance of hid_io to allow for LED updating, so use UefiHidIoFactory to build one. - if let Some(controller) = keyboard_handler.get_controller() { - let hid_io = UefiHidIoFactory::new(context.boot_services, keyboard_handler.get_agent()) - .new_hid_io(controller, false); - if let Ok(hid_io) = hid_io { - if let Err(err) = keyboard_handler.reset(hid_io.as_ref(), extended_verification.into()) { - status = err; - } else { - status = efi::Status::SUCCESS; - } - } - } - } - } + let status = 'reset_processing: { + let Some(keyboard_handler) = (unsafe { context.keyboard_handler.as_mut() }) else { + break 'reset_processing efi::Status::DEVICE_ERROR; + }; + let Some(controller) = keyboard_handler.controller() else { + break 'reset_processing efi::Status::DEVICE_ERROR; + }; + UefiHidIoFactory::new(context.boot_services, keyboard_handler.agent()) + .new_hid_io(controller, false) + .and_then(|hid_io| keyboard_handler.reset(hid_io.as_ref(), extended_verification.into())) + .err() + .unwrap_or(efi::Status::SUCCESS) + }; context.boot_services.restore_tpl(old_tpl); status } @@ -235,26 +230,24 @@ impl SimpleTextInExFfi { return efi::Status::INVALID_PARAMETER; } let context = unsafe { (this as *mut SimpleTextInExFfi).as_mut() }.expect("bad pointer"); - let mut status = efi::Status::SUCCESS; let old_tpl = context.boot_services.raise_tpl(efi::TPL_NOTIFY); - 'read_key_stroke: { + let status = 'read_key_stroke: { let keyboard_handler = unsafe { context.keyboard_handler.as_mut() }; - if let Some(keyboard_handler) = keyboard_handler { - if let Some(key) = keyboard_handler.pop_key() { - unsafe { key_data.write(key) } - } else { - let key = protocols::simple_text_input_ex::KeyData { - key_state: keyboard_handler.get_key_state(), - ..Default::default() - }; - unsafe { key_data.write(key) }; - status = efi::Status::NOT_READY - } + let Some(keyboard_handler) = keyboard_handler else { + break 'read_key_stroke efi::Status::DEVICE_ERROR; + }; + if let Some(key) = keyboard_handler.pop_key() { + unsafe { ptr::write(key_data, key) } + efi::Status::SUCCESS } else { - status = efi::Status::DEVICE_ERROR; - break 'read_key_stroke; + let key = protocols::simple_text_input_ex::KeyData { + key_state: keyboard_handler.get_key_state(), + ..Default::default() + }; + unsafe { ptr::write(key_data, key) }; + efi::Status::NOT_READY } - } + }; context.boot_services.restore_tpl(old_tpl); status } @@ -268,24 +261,22 @@ impl SimpleTextInExFfi { return efi::Status::INVALID_PARAMETER; } let context = unsafe { (this as *mut SimpleTextInExFfi).as_mut() }.expect("bad pointer"); - let mut status = efi::Status::DEVICE_ERROR; let old_tpl = context.boot_services.raise_tpl(efi::TPL_NOTIFY); - '_set_state_processing: { - if let Some(keyboard_handler) = unsafe { context.keyboard_handler.as_mut() } { - if let Some(controller) = keyboard_handler.get_controller() { - let hid_io = UefiHidIoFactory::new(context.boot_services, keyboard_handler.get_agent()) - .new_hid_io(controller, false); - if let Ok(hid_io) = hid_io { - status = efi::Status::SUCCESS; - keyboard_handler.set_key_toggle_state(unsafe { key_toggle_state.read() }); - let result = keyboard_handler.update_leds(hid_io.as_ref()); - if let Err(result) = result { - status = result; - } - } - } - } - } + let status = 'set_state_processing: { + let Some(keyboard_handler) = (unsafe { context.keyboard_handler.as_mut() }) else { + break 'set_state_processing efi::Status::DEVICE_ERROR; + }; + let Some(controller) = keyboard_handler.controller() else { + break 'set_state_processing efi::Status::DEVICE_ERROR; + }; + let Ok(hid_io) = + UefiHidIoFactory::new(context.boot_services, keyboard_handler.agent()).new_hid_io(controller, false) + else { + break 'set_state_processing efi::Status::DEVICE_ERROR; + }; + keyboard_handler.set_key_toggle_state(unsafe { key_toggle_state.read() }); + keyboard_handler.update_leds(hid_io.as_ref()).err().unwrap_or(efi::Status::SUCCESS) + }; context.boot_services.restore_tpl(old_tpl); status } @@ -307,17 +298,16 @@ impl SimpleTextInExFfi { let context = unsafe { (this as *mut SimpleTextInExFfi).as_mut() }.expect("bad pointer"); let old_tpl = context.boot_services.raise_tpl(efi::TPL_NOTIFY); - let status; - { + let status = { if let Some(keyboard_handler) = unsafe { context.keyboard_handler.as_mut() } { let key_data = unsafe { key_data_ptr.read() }; let handle = keyboard_handler.insert_key_notify_callback(key_data, key_notification_function); unsafe { notify_handle.write(handle as *mut c_void) }; - status = efi::Status::SUCCESS; + efi::Status::SUCCESS } else { - status = efi::Status::DEVICE_ERROR; + efi::Status::DEVICE_ERROR } - } + }; context.boot_services.restore_tpl(old_tpl); status } @@ -330,18 +320,16 @@ impl SimpleTextInExFfi { if this.is_null() { return efi::Status::INVALID_PARAMETER; } - let status; let context = unsafe { (this as *mut SimpleTextInExFfi).as_mut() }.expect("bad pointer"); let old_tpl = context.boot_services.raise_tpl(efi::TPL_NOTIFY); - if let Some(keyboard_handler) = unsafe { context.keyboard_handler.as_mut() } { - if let Err(err) = keyboard_handler.remove_key_notify_callback(notification_handle as usize) { - status = err; - } else { - status = efi::Status::SUCCESS; - } + let status = if let Some(keyboard_handler) = unsafe { context.keyboard_handler.as_mut() } { + keyboard_handler + .remove_key_notify_callback(notification_handle as usize) + .err() + .unwrap_or(efi::Status::SUCCESS) } else { - status = efi::Status::DEVICE_ERROR; - } + efi::Status::DEVICE_ERROR + }; context.boot_services.restore_tpl(old_tpl); status } @@ -355,18 +343,16 @@ impl SimpleTextInExFfi { let context = unsafe { (context as *mut SimpleTextInExFfi).as_mut() }.expect("bad pointer"); let old_tpl = context.boot_services.raise_tpl(efi::TPL_NOTIFY); - { - if let Some(keyboard_handler) = unsafe { context.keyboard_handler.as_mut() } { - while let Some(key_data) = keyboard_handler.peek_key() { - if key_data.key.unicode_char == 0 && key_data.key.scan_code == 0 { - // consume (and ignore) the partial stroke. - let _ = keyboard_handler.pop_key(); - continue; - } else { - // valid keystroke - context.boot_services.signal_event(event); - break; - } + if let Some(keyboard_handler) = unsafe { context.keyboard_handler.as_mut() } { + while let Some(key_data) = keyboard_handler.peek_key() { + if key_data.key.unicode_char == 0 && key_data.key.scan_code == 0 { + // consume (and ignore) the partial stroke. + let _ = keyboard_handler.pop_key(); + continue; + } else { + // valid keystroke + context.boot_services.signal_event(event); + break; } } } @@ -376,29 +362,30 @@ impl SimpleTextInExFfi { // Event callback function for handling registered key notifications. Iterates over the queue of keys to be notified, // and invokes the registered callback function for each of those keys. extern "efiapi" fn process_key_notifies(_event: efi::Event, context: *mut c_void) { - if let Some(context) = unsafe { (context as *mut SimpleTextInExFfi).as_mut() } { - loop { - let mut pending_key = None; - let mut pending_callbacks = Vec::new(); - - let old_tpl = context.boot_services.raise_tpl(efi::TPL_NOTIFY); - if let Some(keyboard_handler) = unsafe { context.keyboard_handler.as_mut() } { - (pending_key, pending_callbacks) = keyboard_handler.get_pending_callbacks(); - } else { - debugln!(DEBUG_ERROR, "process_key_notifies event called without a valid keyboard_handler"); - } - context.boot_services.restore_tpl(old_tpl); + let Some(context) = (unsafe { (context as *mut SimpleTextInExFfi).as_mut() }) else { + return; + }; + loop { + let mut pending_key = None; + let mut pending_callbacks = Vec::new(); - //dispatch notifies (if any) at the TPL this event callback was invoked at. - if let Some(mut pending_key) = pending_key { - let key_ptr = &mut pending_key as *mut protocols::simple_text_input_ex::KeyData; - for callback in pending_callbacks { - let _ = callback(key_ptr); - } - } else { - // no pending notifies to process - break; + let old_tpl = context.boot_services.raise_tpl(efi::TPL_NOTIFY); + if let Some(keyboard_handler) = unsafe { context.keyboard_handler.as_mut() } { + (pending_key, pending_callbacks) = keyboard_handler.pending_callbacks(); + } else { + debugln!(DEBUG_ERROR, "process_key_notifies event called without a valid keyboard_handler"); + } + context.boot_services.restore_tpl(old_tpl); + + //dispatch notifies (if any) at the TPL this event callback was invoked at. + if let Some(mut pending_key) = pending_key { + let key_ptr = &mut pending_key as *mut protocols::simple_text_input_ex::KeyData; + for callback in pending_callbacks { + let _ = callback(key_ptr); } + } else { + // no pending notifies to process + break; } } } @@ -409,7 +396,8 @@ mod test { use core::{ ffi::c_void, mem::MaybeUninit, - sync::atomic::{AtomicBool, AtomicPtr}, + ptr, + sync::atomic::{AtomicBool, AtomicPtr, Ordering}, }; use r_efi::{efi, protocols}; @@ -481,13 +469,13 @@ mod test { #[test] fn uninstall_should_uninstall_simple_text_in_interface() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); let boot_services = create_fake_static_boot_service(); // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, interface| { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); efi::Status::SUCCESS }); @@ -495,14 +483,14 @@ mod test { boot_services.expect_open_protocol().returning(|_, protocol, interface, _, _, _| { unsafe { assert_eq!(protocol.read(), protocols::simple_text_input_ex::PROTOCOL_GUID); - interface.write(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst)); + interface.write(CONTEXT_PTR.load(Ordering::SeqCst)); } efi::Status::SUCCESS }); boot_services.expect_uninstall_protocol_interface().returning(|_, protocol, interface| { unsafe { assert_eq!(protocol.read(), protocols::simple_text_input_ex::PROTOCOL_GUID); - assert_eq!(interface, CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst)); + assert_eq!(interface, CONTEXT_PTR.load(Ordering::SeqCst)); } efi::Status::SUCCESS }); @@ -511,20 +499,20 @@ mod test { let mut keyboard_handler = KeyboardHidHandler::new(boot_services, 1 as efi::Handle); SimpleTextInExFfi::install(boot_services, 2 as efi::Handle, &mut keyboard_handler).unwrap(); - assert_ne!(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), core::ptr::null_mut()); + assert_ne!(CONTEXT_PTR.load(Ordering::SeqCst), ptr::null_mut()); SimpleTextInExFfi::uninstall(boot_services, 1 as efi::Handle, 2 as efi::Handle).unwrap(); } #[test] fn reset_should_invoke_reset() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); let boot_services = create_fake_static_boot_service(); // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, interface| { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); efi::Status::SUCCESS }); @@ -564,7 +552,7 @@ mod test { boot_services.expect_uninstall_protocol_interface().returning(|_, protocol, interface| { unsafe { assert_eq!(protocol.read(), protocols::simple_text_input_ex::PROTOCOL_GUID); - assert_eq!(interface, CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst)); + assert_eq!(interface, CONTEXT_PTR.load(Ordering::SeqCst)); } efi::Status::SUCCESS }); @@ -574,10 +562,9 @@ mod test { keyboard_handler.set_controller(Some(2 as efi::Handle)); SimpleTextInExFfi::install(boot_services, 2 as efi::Handle, &mut keyboard_handler).unwrap(); - assert_ne!(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), core::ptr::null_mut()); + assert_ne!(CONTEXT_PTR.load(Ordering::SeqCst), ptr::null_mut()); - let this = - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst) as *mut protocols::simple_text_input_ex::Protocol; + let this = CONTEXT_PTR.load(Ordering::SeqCst) as *mut protocols::simple_text_input_ex::Protocol; SimpleTextInExFfi::simple_text_in_ex_reset(this, efi::Boolean::FALSE); // avoid keyboard drop uninstall flows @@ -586,13 +573,13 @@ mod test { #[test] fn read_key_stroke_should_read_keystrokes() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); let boot_services = create_fake_static_boot_service(); // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, interface| { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); efi::Status::SUCCESS }); @@ -617,7 +604,7 @@ mod test { keyboard_handler.initialize(2 as efi::Handle, &hid_io).unwrap(); SimpleTextInExFfi::install(boot_services, 2 as efi::Handle, &mut keyboard_handler).unwrap(); - assert_ne!(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), core::ptr::null_mut()); + assert_ne!(CONTEXT_PTR.load(Ordering::SeqCst), ptr::null_mut()); let report: &[u8] = &[0x00, 0x00, 0x04, 0x05, 0x06, 0x00, 0x00, 0x00]; keyboard_handler.receive_report(report, &hid_io); @@ -626,8 +613,7 @@ mod test { keyboard_handler.receive_report(report, &hid_io); //read with simple_text_in - let this = - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst) as *mut protocols::simple_text_input_ex::Protocol; + let this = CONTEXT_PTR.load(Ordering::SeqCst) as *mut protocols::simple_text_input_ex::Protocol; let mut input_key: protocols::simple_text_input_ex::KeyData = Default::default(); let status = SimpleTextInExFfi::simple_text_in_ex_read_key_stroke( @@ -740,13 +726,13 @@ mod test { #[test] fn set_state_should_set_state() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); let boot_services = create_fake_static_boot_service(); // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, interface| { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); efi::Status::SUCCESS }); boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); @@ -797,14 +783,13 @@ mod test { keyboard_handler.initialize(2 as efi::Handle, &hid_io).unwrap(); SimpleTextInExFfi::install(boot_services, 2 as efi::Handle, &mut keyboard_handler).unwrap(); - assert_ne!(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), core::ptr::null_mut()); + assert_ne!(CONTEXT_PTR.load(Ordering::SeqCst), ptr::null_mut()); - let this = - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst) as *mut protocols::simple_text_input_ex::Protocol; + let this = CONTEXT_PTR.load(Ordering::SeqCst) as *mut protocols::simple_text_input_ex::Protocol; let mut key_toggle_state: protocols::simple_text_input_ex::KeyToggleState = protocols::simple_text_input_ex::KEY_STATE_EXPOSED; - let status = SimpleTextInExFfi::simple_text_in_ex_set_state(this, core::ptr::addr_of_mut!(key_toggle_state)); + let status = SimpleTextInExFfi::simple_text_in_ex_set_state(this, ptr::addr_of_mut!(key_toggle_state)); assert_eq!(status, efi::Status::SUCCESS); assert_eq!( keyboard_handler.get_key_state().key_toggle_state, @@ -813,7 +798,7 @@ mod test { key_toggle_state = protocols::simple_text_input_ex::CAPS_LOCK_ACTIVE | protocols::simple_text_input_ex::NUM_LOCK_ACTIVE; - let status = SimpleTextInExFfi::simple_text_in_ex_set_state(this, core::ptr::addr_of_mut!(key_toggle_state)); + let status = SimpleTextInExFfi::simple_text_in_ex_set_state(this, ptr::addr_of_mut!(key_toggle_state)); assert_eq!(status, efi::Status::SUCCESS); assert_eq!( keyboard_handler.get_key_state().key_toggle_state, @@ -824,7 +809,7 @@ mod test { #[test] fn register_key_notify_should_register_key() { const NOTIFY_EVENT: efi::Event = 1 as efi::Event; - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); static KEY_NOTIFIED: AtomicBool = AtomicBool::new(false); static KEY2_NOTIFIED: AtomicBool = AtomicBool::new(false); @@ -833,7 +818,7 @@ mod test { // used in install boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, _, _, interface| { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); efi::Status::SUCCESS }); boot_services.expect_locate_protocol().returning(|_, _, _| efi::Status::NOT_FOUND); @@ -846,7 +831,7 @@ mod test { boot_services.expect_create_event_ex().returning(|_, _, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_signal_event().returning(|event| { if event == NOTIFY_EVENT { - SimpleTextInExFfi::process_key_notifies(event, CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst)); + SimpleTextInExFfi::process_key_notifies(event, CONTEXT_PTR.load(Ordering::SeqCst)); } efi::Status::SUCCESS }); @@ -864,14 +849,14 @@ mod test { keyboard_handler.set_notify_event(NOTIFY_EVENT); SimpleTextInExFfi::install(boot_services, 2 as efi::Handle, &mut keyboard_handler).unwrap(); - assert_ne!(CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), core::ptr::null_mut()); + assert_ne!(CONTEXT_PTR.load(Ordering::SeqCst), ptr::null_mut()); extern "efiapi" fn key_notify_callback_a( key_data: *mut protocols::simple_text_input_ex::KeyData, ) -> efi::Status { let key = unsafe { key_data.read() }; assert_eq!(key.key.unicode_char, 'a' as u16); - KEY_NOTIFIED.store(true, core::sync::atomic::Ordering::SeqCst); + KEY_NOTIFIED.store(true, Ordering::SeqCst); efi::Status::SUCCESS } @@ -880,43 +865,42 @@ mod test { ) -> efi::Status { let key = unsafe { key_data.read() }; assert!((key.key.unicode_char == 'a' as u16) || (key.key.unicode_char == 'b' as u16)); - KEY2_NOTIFIED.store(true, core::sync::atomic::Ordering::SeqCst); + KEY2_NOTIFIED.store(true, Ordering::SeqCst); efi::Status::SUCCESS } - let this = - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst) as *mut protocols::simple_text_input_ex::Protocol; + let this = CONTEXT_PTR.load(Ordering::SeqCst) as *mut protocols::simple_text_input_ex::Protocol; let mut key_data: protocols::simple_text_input_ex::KeyData = Default::default(); key_data.key.unicode_char = 'a' as u16; - let mut notify_handle = core::ptr::null_mut(); + let mut notify_handle = ptr::null_mut(); let status = SimpleTextInExFfi::simple_text_in_ex_register_key_notify( this, - core::ptr::addr_of_mut!(key_data), + ptr::addr_of_mut!(key_data), key_notify_callback_a, - core::ptr::addr_of_mut!(notify_handle), + ptr::addr_of_mut!(notify_handle), ); assert_eq!(status, efi::Status::SUCCESS); assert_eq!(notify_handle as usize, 1); - notify_handle = core::ptr::null_mut(); + notify_handle = ptr::null_mut(); let status = SimpleTextInExFfi::simple_text_in_ex_register_key_notify( this, - core::ptr::addr_of_mut!(key_data), + ptr::addr_of_mut!(key_data), key_notify_callback_a_and_b, - core::ptr::addr_of_mut!(notify_handle), + ptr::addr_of_mut!(notify_handle), ); assert_eq!(status, efi::Status::SUCCESS); assert_eq!(notify_handle as usize, 2); - notify_handle = core::ptr::null_mut(); + notify_handle = ptr::null_mut(); key_data.key.unicode_char = 'b' as u16; let status = SimpleTextInExFfi::simple_text_in_ex_register_key_notify( this, - core::ptr::addr_of_mut!(key_data), + ptr::addr_of_mut!(key_data), key_notify_callback_a_and_b, - core::ptr::addr_of_mut!(notify_handle), + ptr::addr_of_mut!(notify_handle), ); assert_eq!(status, efi::Status::SUCCESS); assert_eq!(notify_handle as usize, 3); @@ -929,11 +913,11 @@ mod test { let report: &[u8] = &[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; keyboard_handler.receive_report(report, &hid_io); - assert!(!KEY_NOTIFIED.load(core::sync::atomic::Ordering::SeqCst)); - assert!(KEY2_NOTIFIED.load(core::sync::atomic::Ordering::SeqCst)); + assert!(!KEY_NOTIFIED.load(Ordering::SeqCst)); + assert!(KEY2_NOTIFIED.load(Ordering::SeqCst)); - KEY_NOTIFIED.store(false, core::sync::atomic::Ordering::SeqCst); - KEY2_NOTIFIED.store(false, core::sync::atomic::Ordering::SeqCst); + KEY_NOTIFIED.store(false, Ordering::SeqCst); + KEY2_NOTIFIED.store(false, Ordering::SeqCst); //send 'a' let report: &[u8] = &[0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00]; @@ -943,11 +927,11 @@ mod test { let report: &[u8] = &[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; keyboard_handler.receive_report(report, &hid_io); - assert!(KEY_NOTIFIED.load(core::sync::atomic::Ordering::SeqCst)); - assert!(KEY2_NOTIFIED.load(core::sync::atomic::Ordering::SeqCst)); + assert!(KEY_NOTIFIED.load(Ordering::SeqCst)); + assert!(KEY2_NOTIFIED.load(Ordering::SeqCst)); - KEY_NOTIFIED.store(false, core::sync::atomic::Ordering::SeqCst); - KEY2_NOTIFIED.store(false, core::sync::atomic::Ordering::SeqCst); + KEY_NOTIFIED.store(false, Ordering::SeqCst); + KEY2_NOTIFIED.store(false, Ordering::SeqCst); //remove the 'a'-only callback let status = SimpleTextInExFfi::simple_text_in_ex_unregister_key_notify(this, 1 as *mut c_void); @@ -961,13 +945,13 @@ mod test { let report: &[u8] = &[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; keyboard_handler.receive_report(report, &hid_io); - assert!(!KEY_NOTIFIED.load(core::sync::atomic::Ordering::SeqCst)); - assert!(KEY2_NOTIFIED.load(core::sync::atomic::Ordering::SeqCst)); + assert!(!KEY_NOTIFIED.load(Ordering::SeqCst)); + assert!(KEY2_NOTIFIED.load(Ordering::SeqCst)); } #[test] fn wait_for_key_should_wait_for_key() { - static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); + static CONTEXT_PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); static RECEIVED_EVENT: AtomicBool = AtomicBool::new(false); let boot_services = create_fake_static_boot_service(); @@ -976,7 +960,7 @@ mod test { boot_services.expect_create_event().returning(|_, _, _, _, _| efi::Status::SUCCESS); boot_services.expect_install_protocol_interface().returning(|_, protocol, _, interface| { if unsafe { *protocol } == protocols::simple_text_input_ex::PROTOCOL_GUID { - CONTEXT_PTR.store(interface, core::sync::atomic::Ordering::SeqCst); + CONTEXT_PTR.store(interface, Ordering::SeqCst); } efi::Status::SUCCESS }); @@ -987,7 +971,7 @@ mod test { boot_services.expect_restore_tpl().returning(|_| ()); boot_services.expect_signal_event().returning(|_| { - RECEIVED_EVENT.store(true, core::sync::atomic::Ordering::SeqCst); + RECEIVED_EVENT.store(true, Ordering::SeqCst); efi::Status::SUCCESS }); @@ -1002,12 +986,9 @@ mod test { keyboard_handler.set_controller(Some(2 as efi::Handle)); keyboard_handler.initialize(2 as efi::Handle, &hid_io).unwrap(); - RECEIVED_EVENT.store(false, core::sync::atomic::Ordering::SeqCst); - SimpleTextInExFfi::simple_text_in_ex_wait_for_key( - 3 as efi::Event, - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), - ); - assert!(!RECEIVED_EVENT.load(core::sync::atomic::Ordering::SeqCst)); + RECEIVED_EVENT.store(false, Ordering::SeqCst); + SimpleTextInExFfi::simple_text_in_ex_wait_for_key(3 as efi::Event, CONTEXT_PTR.load(Ordering::SeqCst)); + assert!(!RECEIVED_EVENT.load(Ordering::SeqCst)); // press the a key let report: &[u8] = &[0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00]; @@ -1017,11 +998,8 @@ mod test { let report: &[u8] = &[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; keyboard_handler.receive_report(report, &hid_io); - SimpleTextInExFfi::simple_text_in_ex_wait_for_key( - 3 as efi::Event, - CONTEXT_PTR.load(core::sync::atomic::Ordering::SeqCst), - ); - assert!(RECEIVED_EVENT.load(core::sync::atomic::Ordering::SeqCst)); + SimpleTextInExFfi::simple_text_in_ex_wait_for_key(3 as efi::Event, CONTEXT_PTR.load(Ordering::SeqCst)); + assert!(RECEIVED_EVENT.load(Ordering::SeqCst)); // avoid keyboard drop uninstall flows keyboard_handler.set_controller(None); diff --git a/HidPkg/UefiHidDxeV2/src/lib.rs b/HidPkg/UefiHidDxeV2/src/lib.rs index 5dca990737..ad50e51d38 100644 --- a/HidPkg/UefiHidDxeV2/src/lib.rs +++ b/HidPkg/UefiHidDxeV2/src/lib.rs @@ -30,12 +30,14 @@ pub mod hid_io; pub mod keyboard; pub mod pointer; -use boot_services::StandardUefiBootServices; -use core::sync::atomic::AtomicPtr; +use core::{ptr, sync::atomic::AtomicPtr}; + use r_efi::efi; +use boot_services::StandardUefiBootServices; + /// Global instance of UEFI Boot Services. pub static BOOT_SERVICES: StandardUefiBootServices = StandardUefiBootServices::new(); /// Global instance of UEFI Runtime Services. -pub static RUNTIME_SERVICES: AtomicPtr = AtomicPtr::new(core::ptr::null_mut()); +pub static RUNTIME_SERVICES: AtomicPtr = AtomicPtr::new(ptr::null_mut()); diff --git a/HidPkg/UefiHidDxeV2/src/main.rs b/HidPkg/UefiHidDxeV2/src/main.rs index c373fe8791..1044933717 100644 --- a/HidPkg/UefiHidDxeV2/src/main.rs +++ b/HidPkg/UefiHidDxeV2/src/main.rs @@ -15,9 +15,8 @@ #[cfg(target_os = "uefi")] mod uefi_entry { extern crate alloc; - use core::{panic::PanicInfo, sync::atomic::Ordering}; - use alloc::{boxed::Box, vec::Vec}; + use core::{panic::PanicInfo, sync::atomic::Ordering}; use r_efi::{efi, system}; diff --git a/HidPkg/UefiHidDxeV2/src/pointer/mod.rs b/HidPkg/UefiHidDxeV2/src/pointer.rs similarity index 97% rename from HidPkg/UefiHidDxeV2/src/pointer/mod.rs rename to HidPkg/UefiHidDxeV2/src/pointer.rs index 78e7a07afd..479f7c17d3 100644 --- a/HidPkg/UefiHidDxeV2/src/pointer/mod.rs +++ b/HidPkg/UefiHidDxeV2/src/pointer.rs @@ -15,21 +15,21 @@ use alloc::{ collections::{BTreeMap, BTreeSet}, vec::Vec, }; + +use r_efi::{efi, protocols}; + use hidparser::{ report_data_types::{ReportId, Usage}, ReportDescriptor, ReportField, VariableField, }; -use r_efi::{efi, protocols}; - use rust_advanced_logger_dxe::{debugln, function, DEBUG_ERROR, DEBUG_VERBOSE}; +use self::absolute_pointer::PointerContext; use crate::{ boot_services::UefiBootServices, hid_io::{HidIo, HidReportReceiver}, }; -use self::absolute_pointer::PointerContext; - // Usages supported by this module. const GENERIC_DESKTOP_X: u32 = 0x00010030; const GENERIC_DESKTOP_Y: u32 = 0x00010031; @@ -147,7 +147,6 @@ impl PointerHidHandler { self.input_reports.insert(report_data.report_id, report_data); } } - if !self.input_reports.is_empty() { Ok(()) } else { diff --git a/HidPkg/UefiHidDxeV2/src/pointer/absolute_pointer.rs b/HidPkg/UefiHidDxeV2/src/pointer/absolute_pointer.rs index 5c00c6fb19..6b42ff72c1 100644 --- a/HidPkg/UefiHidDxeV2/src/pointer/absolute_pointer.rs +++ b/HidPkg/UefiHidDxeV2/src/pointer/absolute_pointer.rs @@ -8,15 +8,15 @@ //! SPDX-License-Identifier: BSD-2-Clause-Patent //! use alloc::boxed::Box; -use core::ffi::c_void; +use core::{ffi::c_void, ptr}; -use hidparser::report_data_types::Usage; use r_efi::{efi, protocols}; -use rust_advanced_logger_dxe::{debugln, DEBUG_ERROR, DEBUG_INFO, DEBUG_WARN}; -use crate::boot_services::UefiBootServices; +use hidparser::report_data_types::Usage; +use rust_advanced_logger_dxe::{debugln, DEBUG_ERROR, DEBUG_INFO, DEBUG_WARN}; use super::{PointerHidHandler, BUTTON_MAX, BUTTON_MIN, DIGITIZER_SWITCH_MAX, DIGITIZER_SWITCH_MIN}; +use crate::boot_services::UefiBootServices; // FFI context // Safety: a pointer to PointerHidHandler is included in the context so that it can be reclaimed in the absolute_pointer @@ -57,7 +57,7 @@ impl PointerContext { reset: Self::absolute_pointer_reset, get_state: Self::absolute_pointer_get_state, mode: Box::into_raw(Box::new(Self::initialize_mode(pointer_handler))), - wait_for_input: core::ptr::null_mut(), + wait_for_input: ptr::null_mut(), }, boot_services, pointer_handler: pointer_handler as *mut PointerHidHandler, @@ -66,13 +66,13 @@ impl PointerContext { let absolute_pointer_ptr = Box::into_raw(Box::new(pointer_ctx)); // create event for wait_for_input. - let mut wait_for_pointer_input_event: efi::Event = core::ptr::null_mut(); + let mut wait_for_pointer_input_event: efi::Event = ptr::null_mut(); let status = boot_services.create_event( efi::EVT_NOTIFY_WAIT, efi::TPL_NOTIFY, Some(Self::wait_for_pointer), absolute_pointer_ptr as *mut c_void, - core::ptr::addr_of_mut!(wait_for_pointer_input_event), + ptr::addr_of_mut!(wait_for_pointer_input_event), ); if status.is_error() { drop(unsafe { Box::from_raw(absolute_pointer_ptr) }); @@ -84,7 +84,7 @@ impl PointerContext { // install the absolute_pointer protocol. let mut controller = controller; let status = boot_services.install_protocol_interface( - core::ptr::addr_of_mut!(controller), + ptr::addr_of_mut!(controller), &protocols::absolute_pointer::PROTOCOL_GUID as *const efi::Guid as *mut efi::Guid, efi::NATIVE_INTERFACE, absolute_pointer_ptr as *mut c_void, @@ -148,11 +148,11 @@ impl PointerContext { agent: efi::Handle, controller: efi::Handle, ) -> Result<(), efi::Status> { - let mut absolute_pointer_ptr: *mut PointerContext = core::ptr::null_mut(); + let mut absolute_pointer_ptr: *mut PointerContext = ptr::null_mut(); let status = boot_services.open_protocol( controller, &protocols::absolute_pointer::PROTOCOL_GUID as *const efi::Guid as *mut efi::Guid, - core::ptr::addr_of_mut!(absolute_pointer_ptr) as *mut *mut c_void, + ptr::addr_of_mut!(absolute_pointer_ptr) as *mut *mut c_void, agent, controller, efi::OPEN_PROTOCOL_GET_PROTOCOL, @@ -177,7 +177,7 @@ impl PointerContext { debugln!(DEBUG_ERROR, "Failed to uninstall absolute_pointer interface, status: {:x?}", status); unsafe { - (*absolute_pointer_ptr).pointer_handler = core::ptr::null_mut(); + (*absolute_pointer_ptr).pointer_handler = ptr::null_mut(); } //return without tearing down the context. return Err(status); @@ -193,7 +193,7 @@ impl PointerContext { //and return error based on observing pointer_handler is null. debugln!(DEBUG_ERROR, "Failed to close absolute_pointer.wait_for_input event, status: {:x?}", status); unsafe { - (*absolute_pointer_ptr).pointer_handler = core::ptr::null_mut(); + (*absolute_pointer_ptr).pointer_handler = ptr::null_mut(); } return Err(status); } @@ -290,18 +290,14 @@ impl PointerContext { #[cfg(test)] mod test { - - use core::ffi::c_void; - - use r_efi::{efi, protocols}; - use super::*; - use crate::{ boot_services::MockUefiBootServices, hid_io::{HidReportReceiver, MockHidIo}, pointer::CENTER, }; + use core::ffi::c_void; + use r_efi::{efi, protocols}; static MOUSE_REPORT_DESCRIPTOR: &[u8] = &[ 0x05, 0x01, // USAGE_PAGE (Generic Desktop) @@ -349,14 +345,14 @@ mod test { const CONTROLLER_HANDLE: efi::Handle = 0x02 as efi::Handle; const POINTER_EVENT: efi::Event = 0x03 as efi::Event; - static mut ABS_PTR_INTERFACE: *mut c_void = core::ptr::null_mut(); - static mut EVENT_CONTEXT: *mut c_void = core::ptr::null_mut(); + static mut ABS_PTR_INTERFACE: *mut c_void = ptr::null_mut(); + static mut EVENT_CONTEXT: *mut c_void = ptr::null_mut(); static mut EVENT_SIGNALED: bool = false; // expected on PointerHidHandler::initialize(). boot_services.expect_create_event().returning(|_, _, wait_for_ptr, context, event_ptr| { assert!(wait_for_ptr == Some(PointerContext::wait_for_pointer)); - assert_ne!(context, core::ptr::null_mut()); + assert_ne!(context, ptr::null_mut()); unsafe { EVENT_CONTEXT = context; event_ptr.write(POINTER_EVENT); @@ -428,13 +424,13 @@ mod test { const CONTROLLER_HANDLE: efi::Handle = 0x02 as efi::Handle; const EVENT_HANDLE: efi::Handle = 0x03 as efi::Handle; - static mut ABS_PTR_INTERFACE: *mut c_void = core::ptr::null_mut(); - static mut EVENT_CONTEXT: *mut c_void = core::ptr::null_mut(); + static mut ABS_PTR_INTERFACE: *mut c_void = ptr::null_mut(); + static mut EVENT_CONTEXT: *mut c_void = ptr::null_mut(); // expected on PointerHidHandler::initialize(). boot_services.expect_create_event().returning(|_, _, wait_for_ptr, context, event_ptr| { assert!(wait_for_ptr == Some(PointerContext::wait_for_pointer)); - assert_ne!(context, core::ptr::null_mut()); + assert_ne!(context, ptr::null_mut()); unsafe { EVENT_CONTEXT = context; event_ptr.write(EVENT_HANDLE); @@ -513,13 +509,13 @@ mod test { const CONTROLLER_HANDLE: efi::Handle = 0x02 as efi::Handle; const EVENT_HANDLE: efi::Handle = 0x03 as efi::Handle; - static mut ABS_PTR_INTERFACE: *mut c_void = core::ptr::null_mut(); - static mut EVENT_CONTEXT: *mut c_void = core::ptr::null_mut(); + static mut ABS_PTR_INTERFACE: *mut c_void = ptr::null_mut(); + static mut EVENT_CONTEXT: *mut c_void = ptr::null_mut(); // expected on PointerHidHandler::initialize(). boot_services.expect_create_event().returning(|_, _, wait_for_ptr, context, event_ptr| { assert!(wait_for_ptr == Some(PointerContext::wait_for_pointer)); - assert_ne!(context, core::ptr::null_mut()); + assert_ne!(context, ptr::null_mut()); unsafe { EVENT_CONTEXT = context; event_ptr.write(EVENT_HANDLE);