Skip to content

Commit 8cef9a9

Browse files
author
Sanjukta Mitra
committed
Removing magic numbers from drivers sample
1 parent b9aa8bb commit 8cef9a9

File tree

4 files changed

+44
-18
lines changed

4 files changed

+44
-18
lines changed

general/echo/kmdf/driver/DriverSync/src/device.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ use crate::{
3030
WDF_REQUEST_CONTEXT_TYPE_INFO,
3131
};
3232

33+
// Define constants for magic numbers
34+
const DEFAULT_PRIVATE_DATA: u32 = 0;
35+
const TIMER_DUE_TIME_MS: i64 = -100 * 10000;
36+
3337
/// Worker routine called to create a device and its software resources.
3438
///
3539
/// # Arguments:
@@ -108,7 +112,7 @@ pub fn echo_device_create(mut device_init: &mut WDFDEVICE_INIT) -> NTSTATUS {
108112
// it will return NULL and assert if run under framework verifier mode.
109113
let device_context: *mut DeviceContext =
110114
unsafe { wdf_object_get_device_context(device as WDFOBJECT) };
111-
unsafe { (*device_context).private_device_data = 0 };
115+
unsafe { (*device_context).private_device_data = DEFAULT_PRIVATE_DATA };
112116

113117
// Create a device interface so that application can find and talk
114118
// to us.
@@ -162,7 +166,7 @@ extern "C" fn echo_evt_device_self_managed_io_start(device: WDFDEVICE) -> NTSTAT
162166
// into low power state.
163167
unsafe { call_unsafe_wdf_function_binding!(WdfIoQueueStart, queue) };
164168

165-
let due_time: i64 = -(100) * (10000);
169+
let due_time: i64 = TIMER_DUE_TIME_MS;
166170

167171
let _ = unsafe { (*queue_context).timer.start(due_time) };
168172

general/echo/kmdf/driver/DriverSync/src/queue.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,18 @@ use crate::{
4444
WDF_TIMER_CONFIG_SIZE,
4545
};
4646

47-
/// Set max write length for testing
47+
/// Set max write length for testing (40KB)
4848
const MAX_WRITE_LENGTH: usize = 1024 * 40;
4949

50-
/// Set timer period in ms
50+
/// Set timer period in ms (10 seconds)
5151
const TIMER_PERIOD: u32 = 1000 * 10;
5252

53+
/// Increment value for atomic operations
54+
const ATOMIC_INCREMENT_VALUE: i32 = 1;
55+
56+
/// Floor value for zero-based operations
57+
const ZERO_FLOOR: i32 = 0;
58+
5359
/// This routine will interlock increment a value only if the current value
5460
/// is greater then the floor value.
5561
///
@@ -79,7 +85,7 @@ fn echo_interlocked_increment_floor(target: &AtomicI32, floor: i32) -> i32 {
7985
//
8086
match target.compare_exchange(
8187
current_value,
82-
current_value + 1,
88+
current_value + ATOMIC_INCREMENT_VALUE,
8389
Ordering::SeqCst,
8490
Ordering::SeqCst,
8591
) {
@@ -91,7 +97,7 @@ fn echo_interlocked_increment_floor(target: &AtomicI32, floor: i32) -> i32 {
9197
}
9298
}
9399

94-
current_value + 1
100+
current_value + ATOMIC_INCREMENT_VALUE
95101
}
96102

97103
/// Increment the value only if it is currently > 0.
@@ -104,7 +110,7 @@ fn echo_interlocked_increment_floor(target: &AtomicI32, floor: i32) -> i32 {
104110
///
105111
/// Upon success, a value > 0. Upon failure, a value <= 0.
106112
fn echo_interlocked_increment_gtzero(target: &AtomicI32) -> i32 {
107-
echo_interlocked_increment_floor(target, 0)
113+
echo_interlocked_increment_floor(target, ZERO_FLOOR)
108114
}
109115

110116
/// The I/O dispatch callbacks for the frameworks device object
@@ -208,7 +214,7 @@ pub unsafe fn echo_queue_initialize(device: WDFDEVICE) -> NTSTATUS {
208214
EvtTimerFunc: Some(echo_evt_timer_func),
209215
Period: TIMER_PERIOD,
210216
AutomaticSerialization: u8::from(true),
211-
TolerableDelay: 0,
217+
TolerableDelay: TOLERABLE_DELAY,
212218
..WDF_TIMER_CONFIG::default()
213219
};
214220

@@ -266,7 +272,7 @@ fn echo_decrement_request_cancel_ownership_count(request_context: *mut RequestCo
266272
.fetch_sub(1, Ordering::SeqCst)
267273
};
268274

269-
result - 1 == 0
275+
result - RESULT_SUCCESS_THRESHOLD == 0
270276
}
271277

272278
/// 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) {
727733
}
728734
}
729735
}
736+
737+
// Define constants for magic numbers
738+
const TOLERABLE_DELAY: u32 = 0;
739+
const RESULT_SUCCESS_THRESHOLD: i32 = 1;

general/echo/kmdf/exe/src/main.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,15 @@ use windows_sys::Win32::{
5959
},
6060
};
6161

62+
// Define constants for magic numbers
63+
const READER_TYPE: u32 = 1;
64+
const WRITER_TYPE: u32 = 2;
65+
const NUM_ASYNCH_IO: usize = 100;
66+
const BUFFER_SIZE: usize = 40 * 1024;
67+
const TEST_SIZE_SMALL: usize = 512;
68+
const TEST_SIZE_LARGE: usize = 30 * 1024;
69+
const NULL_TERMINATOR: u16 = 0;
70+
6271
#[derive(Default, Debug)]
6372
struct Globals {
6473
perform_async_io: bool,
@@ -69,10 +78,6 @@ struct Globals {
6978

7079
static GLOBAL_DATA: Lazy<RwLock<Globals>> = Lazy::new(|| RwLock::new(Globals::default()));
7180
static GUID_DEVINTERFACE_ECHO: Uuid = uuid!("CDC35B6E-0BE4-4936-BF5F-5537380A7C1A");
72-
static READER_TYPE: u32 = 1;
73-
static WRITER_TYPE: u32 = 2;
74-
static NUM_ASYNCH_IO: usize = 100;
75-
static BUFFER_SIZE: usize = 40 * 1024;
7681

7782
fn main() -> Result<(), Box<dyn Error>> {
7883
let argument_vector: Vec<String> = env::args().collect();
@@ -111,7 +116,7 @@ Exit the app anytime by pressing Ctrl-C
111116
drop(globals);
112117

113118
let h_device: HANDLE;
114-
path_vec.push(0);
119+
path_vec.push(NULL_TERMINATOR);
115120
let path = path_vec.as_ptr();
116121

117122
// SAFETY:
@@ -153,9 +158,9 @@ Exit the app anytime by pressing Ctrl-C
153158

154159
h.join().unwrap().unwrap();
155160
} else {
156-
perform_write_read_test(h_device, 512)?;
161+
perform_write_read_test(h_device, u32::try_from(TEST_SIZE_SMALL).unwrap())?;
157162

158-
perform_write_read_test(h_device, 30 * 1024)?;
163+
perform_write_read_test(h_device, u32::try_from(TEST_SIZE_LARGE).unwrap())?;
159164
}
160165

161166
Ok(())

tools/dv/kmdf/fail_driver_pool_leak/src/driver.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ use wdk_sys::{
2525

2626
use crate::{GLOBAL_BUFFER, GUID_DEVINTERFACE};
2727

28+
// Define constants for magic numbers
29+
const POOL_ALLOCATION_SIZE: usize = 64;
30+
const POOL_TAG: u32 = 's' as u32;
31+
2832
/// `DriverEntry` initializes the driver and is the first routine called by the
2933
/// system after the driver is loaded. `DriverEntry` specifies the other entry
3034
/// points in the function driver, such as `EvtDevice` and `DriverUnload`.
@@ -147,8 +151,11 @@ extern "C" fn evt_driver_device_add(
147151
// Global buffer. This pool of memory is intentionally not freed by
148152
// the driver.
149153
unsafe {
150-
const LENGTH: usize = 64;
151-
GLOBAL_BUFFER = ExAllocatePool2(POOL_FLAG_NON_PAGED, LENGTH as SIZE_T, 's' as u32);
154+
GLOBAL_BUFFER = ExAllocatePool2(
155+
POOL_FLAG_NON_PAGED,
156+
POOL_ALLOCATION_SIZE as SIZE_T,
157+
POOL_TAG,
158+
);
152159
}
153160

154161
nt_status = unsafe {

0 commit comments

Comments
 (0)