Skip to content

Commit

Permalink
UEFI HID DXE V2 to improve Idiomatic Rust patterns (#504)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
magravel authored Jul 29, 2024
1 parent 013d135 commit e60e990
Show file tree
Hide file tree
Showing 11 changed files with 337 additions and 380 deletions.
1 change: 1 addition & 0 deletions HidPkg/UefiHidDxeV2/src/boot_services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 6 additions & 8 deletions HidPkg/UefiHidDxeV2/src/hid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
36 changes: 18 additions & 18 deletions HidPkg/UefiHidDxeV2/src/hid_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -97,7 +96,7 @@ impl UefiHidIo {
controller: efi::Handle,
owned: bool,
) -> Result<Self, efi::Status> {
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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -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 => (),
Expand Down Expand Up @@ -227,6 +226,7 @@ impl HidIo for UefiHidIo {
mod test {
use core::{
ffi::c_void,
ptr,
slice::{from_raw_parts, from_raw_parts_mut},
};

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) };

Expand All @@ -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);
Expand All @@ -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
}
Expand All @@ -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);
Expand Down
Loading

0 comments on commit e60e990

Please sign in to comment.