From 8cef9a9c1c7d679f0da6dd587738360e17e9e016 Mon Sep 17 00:00:00 2001 From: Sanjukta Mitra Date: Mon, 7 Jul 2025 15:23:44 -0700 Subject: [PATCH] Removing magic numbers from drivers sample --- .../echo/kmdf/driver/DriverSync/src/device.rs | 8 +++++-- .../echo/kmdf/driver/DriverSync/src/queue.rs | 24 +++++++++++++------ general/echo/kmdf/exe/src/main.rs | 19 +++++++++------ .../kmdf/fail_driver_pool_leak/src/driver.rs | 11 +++++++-- 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/general/echo/kmdf/driver/DriverSync/src/device.rs b/general/echo/kmdf/driver/DriverSync/src/device.rs index e25e274..1d35508 100644 --- a/general/echo/kmdf/driver/DriverSync/src/device.rs +++ b/general/echo/kmdf/driver/DriverSync/src/device.rs @@ -30,6 +30,10 @@ use crate::{ WDF_REQUEST_CONTEXT_TYPE_INFO, }; +// Define constants for magic numbers +const DEFAULT_PRIVATE_DATA: u32 = 0; +const TIMER_DUE_TIME_MS: i64 = -100 * 10000; + /// Worker routine called to create a device and its software resources. /// /// # Arguments: @@ -108,7 +112,7 @@ pub fn echo_device_create(mut device_init: &mut WDFDEVICE_INIT) -> NTSTATUS { // it will return NULL and assert if run under framework verifier mode. let device_context: *mut DeviceContext = unsafe { wdf_object_get_device_context(device as WDFOBJECT) }; - unsafe { (*device_context).private_device_data = 0 }; + unsafe { (*device_context).private_device_data = DEFAULT_PRIVATE_DATA }; // Create a device interface so that application can find and talk // to us. @@ -162,7 +166,7 @@ extern "C" fn echo_evt_device_self_managed_io_start(device: WDFDEVICE) -> NTSTAT // into low power state. unsafe { call_unsafe_wdf_function_binding!(WdfIoQueueStart, queue) }; - let due_time: i64 = -(100) * (10000); + let due_time: i64 = TIMER_DUE_TIME_MS; let _ = unsafe { (*queue_context).timer.start(due_time) }; diff --git a/general/echo/kmdf/driver/DriverSync/src/queue.rs b/general/echo/kmdf/driver/DriverSync/src/queue.rs index dc9c8e3..aa1ccf0 100644 --- a/general/echo/kmdf/driver/DriverSync/src/queue.rs +++ b/general/echo/kmdf/driver/DriverSync/src/queue.rs @@ -44,12 +44,18 @@ use crate::{ WDF_TIMER_CONFIG_SIZE, }; -/// Set max write length for testing +/// Set max write length for testing (40KB) const MAX_WRITE_LENGTH: usize = 1024 * 40; -/// Set timer period in ms +/// Set timer period in ms (10 seconds) const TIMER_PERIOD: u32 = 1000 * 10; +/// Increment value for atomic operations +const ATOMIC_INCREMENT_VALUE: i32 = 1; + +/// Floor value for zero-based operations +const ZERO_FLOOR: i32 = 0; + /// This routine will interlock increment a value only if the current value /// is greater then the floor value. /// @@ -79,7 +85,7 @@ fn echo_interlocked_increment_floor(target: &AtomicI32, floor: i32) -> i32 { // match target.compare_exchange( current_value, - current_value + 1, + current_value + ATOMIC_INCREMENT_VALUE, Ordering::SeqCst, Ordering::SeqCst, ) { @@ -91,7 +97,7 @@ fn echo_interlocked_increment_floor(target: &AtomicI32, floor: i32) -> i32 { } } - current_value + 1 + current_value + ATOMIC_INCREMENT_VALUE } /// Increment the value only if it is currently > 0. @@ -104,7 +110,7 @@ fn echo_interlocked_increment_floor(target: &AtomicI32, floor: i32) -> i32 { /// /// Upon success, a value > 0. Upon failure, a value <= 0. fn echo_interlocked_increment_gtzero(target: &AtomicI32) -> i32 { - echo_interlocked_increment_floor(target, 0) + echo_interlocked_increment_floor(target, ZERO_FLOOR) } /// The I/O dispatch callbacks for the frameworks device object @@ -208,7 +214,7 @@ pub unsafe fn echo_queue_initialize(device: WDFDEVICE) -> NTSTATUS { EvtTimerFunc: Some(echo_evt_timer_func), Period: TIMER_PERIOD, AutomaticSerialization: u8::from(true), - TolerableDelay: 0, + TolerableDelay: TOLERABLE_DELAY, ..WDF_TIMER_CONFIG::default() }; @@ -266,7 +272,7 @@ fn echo_decrement_request_cancel_ownership_count(request_context: *mut RequestCo .fetch_sub(1, Ordering::SeqCst) }; - result - 1 == 0 + result - RESULT_SUCCESS_THRESHOLD == 0 } /// Attempts to increment the request ownership count so that it cannot be @@ -727,3 +733,7 @@ unsafe extern "C" fn echo_evt_timer_func(timer: WDFTIMER) { } } } + +// Define constants for magic numbers +const TOLERABLE_DELAY: u32 = 0; +const RESULT_SUCCESS_THRESHOLD: i32 = 1; diff --git a/general/echo/kmdf/exe/src/main.rs b/general/echo/kmdf/exe/src/main.rs index bcdf6db..1d1a88d 100644 --- a/general/echo/kmdf/exe/src/main.rs +++ b/general/echo/kmdf/exe/src/main.rs @@ -59,6 +59,15 @@ use windows_sys::Win32::{ }, }; +// Define constants for magic numbers +const READER_TYPE: u32 = 1; +const WRITER_TYPE: u32 = 2; +const NUM_ASYNCH_IO: usize = 100; +const BUFFER_SIZE: usize = 40 * 1024; +const TEST_SIZE_SMALL: usize = 512; +const TEST_SIZE_LARGE: usize = 30 * 1024; +const NULL_TERMINATOR: u16 = 0; + #[derive(Default, Debug)] struct Globals { perform_async_io: bool, @@ -69,10 +78,6 @@ struct Globals { static GLOBAL_DATA: Lazy> = Lazy::new(|| RwLock::new(Globals::default())); static GUID_DEVINTERFACE_ECHO: Uuid = uuid!("CDC35B6E-0BE4-4936-BF5F-5537380A7C1A"); -static READER_TYPE: u32 = 1; -static WRITER_TYPE: u32 = 2; -static NUM_ASYNCH_IO: usize = 100; -static BUFFER_SIZE: usize = 40 * 1024; fn main() -> Result<(), Box> { let argument_vector: Vec = env::args().collect(); @@ -111,7 +116,7 @@ Exit the app anytime by pressing Ctrl-C drop(globals); let h_device: HANDLE; - path_vec.push(0); + path_vec.push(NULL_TERMINATOR); let path = path_vec.as_ptr(); // SAFETY: @@ -153,9 +158,9 @@ Exit the app anytime by pressing Ctrl-C h.join().unwrap().unwrap(); } else { - perform_write_read_test(h_device, 512)?; + perform_write_read_test(h_device, u32::try_from(TEST_SIZE_SMALL).unwrap())?; - perform_write_read_test(h_device, 30 * 1024)?; + perform_write_read_test(h_device, u32::try_from(TEST_SIZE_LARGE).unwrap())?; } Ok(()) diff --git a/tools/dv/kmdf/fail_driver_pool_leak/src/driver.rs b/tools/dv/kmdf/fail_driver_pool_leak/src/driver.rs index 044ae2e..074aa78 100644 --- a/tools/dv/kmdf/fail_driver_pool_leak/src/driver.rs +++ b/tools/dv/kmdf/fail_driver_pool_leak/src/driver.rs @@ -25,6 +25,10 @@ use wdk_sys::{ use crate::{GLOBAL_BUFFER, GUID_DEVINTERFACE}; +// Define constants for magic numbers +const POOL_ALLOCATION_SIZE: usize = 64; +const POOL_TAG: u32 = 's' as u32; + /// `DriverEntry` initializes the driver and is the first routine called by the /// system after the driver is loaded. `DriverEntry` specifies the other entry /// points in the function driver, such as `EvtDevice` and `DriverUnload`. @@ -147,8 +151,11 @@ extern "C" fn evt_driver_device_add( // Global buffer. This pool of memory is intentionally not freed by // the driver. unsafe { - const LENGTH: usize = 64; - GLOBAL_BUFFER = ExAllocatePool2(POOL_FLAG_NON_PAGED, LENGTH as SIZE_T, 's' as u32); + GLOBAL_BUFFER = ExAllocatePool2( + POOL_FLAG_NON_PAGED, + POOL_ALLOCATION_SIZE as SIZE_T, + POOL_TAG, + ); } nt_status = unsafe {