refactor: remove magic numbers (#64)

## Summary

Decompose magic numbers into `const`'s adding relevant documentation.

---------

Signed-off-by: Alan Ngo <ngoalan@microsoft.com>

Author: Alan632

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

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // License: MIT OR Apache-2.0
 
+use core::time::Duration;
+
 use wdk::{nt_success, paged_code, println};
 use wdk_sys::{
     call_unsafe_wdf_function_binding,
@@ -146,6 +148,24 @@ pub fn echo_device_create(mut device_init: &mut WDFDEVICE_INIT) -> NTSTATUS {
 ///
 /// * `NTSTATUS` - Failures will result in the device stack being torn down.
 extern "C" fn echo_evt_device_self_managed_io_start(device: WDFDEVICE) -> NTSTATUS {
+    /// 100ms relative due time for a WDF timer, in system time units.
+    #[allow(
+        clippy::cast_possible_truncation,
+        reason = "100ms in 100-nanosecond units is known to fit in i64"
+    )]
+    const WDF_TIMER_DUE_TIME_100_MS: i64 = {
+        // System time units are 100-nanosecond intervals.
+        const SYSTEM_TIME_UNITS: u128 = Duration::from_millis(100).as_nanos() / 100;
+        const {
+            assert!(
+                SYSTEM_TIME_UNITS <= i64::MAX as u128,
+                "SYSTEM_TIME_UNITS should fit in i64"
+            );
+        };
+        // Negative marks the value as a relative (rather than absolute) timeout.
+        -(SYSTEM_TIME_UNITS as i64)
+    };
+
     // Restart the queue and the periodic timer. We stopped them before going
     // into low power state.
     let queue: WDFQUEUE;
@@ -162,7 +182,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 = WDF_TIMER_DUE_TIME_100_MS;
 
     let _ = unsafe { (*queue_context).timer.start(due_time) };
 

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

@@ -177,9 +177,15 @@ fn echo_print_driver_version() -> NTSTATUS {
         call_unsafe_wdf_function_binding!(WdfDriverIsVersionAvailable, driver, &raw mut ver)
     } > 0
     {
-        println!("Yes, framework version is 1.0");
+        println!(
+            "Yes, framework version is {}.{}",
+            ver.MajorVersion, ver.MinorVersion
+        );
     } else {
-        println!("No, framework version is not 1.0");
+        println!(
+            "No, framework version is not {}.{}",
+            ver.MajorVersion, ver.MinorVersion
+        );
     }
 
     STATUS_SUCCESS

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

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // License: MIT OR Apache-2.0
 
-use core::sync::atomic::Ordering;
+use core::{sync::atomic::Ordering, time::Duration};
 
 use wdk::{nt_success, paged_code, println, wdf};
 use wdk_sys::{
@@ -44,12 +44,6 @@ use crate::{
     WDF_TIMER_CONFIG_SIZE,
 };
 
-/// Set max write length for testing
-const MAX_WRITE_LENGTH: usize = 1024 * 40;
-
-/// Set timer period in ms
-const TIMER_PERIOD: u32 = 1000 * 10;
-
 /// This routine will interlock increment a value only if the current value
 /// is greater then the floor value.
 ///
@@ -130,6 +124,19 @@ fn echo_interlocked_increment_gtzero(target: &AtomicI32) -> i32 {
 /// * `NTSTATUS`
 #[link_section = "PAGE"]
 pub unsafe fn echo_queue_initialize(device: WDFDEVICE) -> NTSTATUS {
+    #[allow(
+        clippy::cast_possible_truncation,
+        reason = "10 seconds in millisecond units is known to fit in u32"
+    )]
+    const TIMER_PERIOD_MS: u32 = {
+        const MILLIS: u128 = Duration::from_secs(10).as_millis();
+        const {
+            assert!(MILLIS <= u32::MAX as u128, "10,000 should fit in u32");
+        };
+
+        MILLIS as u32
+    };
+
     paged_code!();
 
     let mut queue = WDF_NO_HANDLE as WDFQUEUE;
@@ -206,7 +213,7 @@ pub unsafe fn echo_queue_initialize(device: WDFDEVICE) -> NTSTATUS {
     let mut timer_config = WDF_TIMER_CONFIG {
         Size: WDF_TIMER_CONFIG_SIZE,
         EvtTimerFunc: Some(echo_evt_timer_func),
-        Period: TIMER_PERIOD,
+        Period: TIMER_PERIOD_MS,
         AutomaticSerialization: u8::from(true),
         TolerableDelay: 0,
         ..WDF_TIMER_CONFIG::default()
@@ -518,6 +525,15 @@ extern "C" fn echo_evt_io_read(queue: WDFQUEUE, request: WDFREQUEST, mut length:
 ///
 /// * `VOID`
 extern "C" fn echo_evt_io_write(queue: WDFQUEUE, request: WDFREQUEST, length: usize) {
+    /// Number of bytes in one kilobyte.
+    const BYTES_PER_KB: usize = 1024;
+    /// Max write length, in bytes, for testing
+    const MAX_WRITE_LENGTH: usize = 40 * BYTES_PER_KB;
+
+    /// Non-zero char literal (of one to four chars) for pool tag used in
+    /// `ExAllocatePool2`
+    const MEMORY_TAG: u32 = u32::from_be_bytes(*b"sam1");
+
     let mut memory = WDF_NO_HANDLE as WDFMEMORY;
     let mut status: NTSTATUS;
     let queue_context = unsafe { queue_get_context(queue as WDFOBJECT) };
@@ -564,9 +580,8 @@ extern "C" fn echo_evt_io_write(queue: WDFQUEUE, request: WDFREQUEST, length: us
             (*queue_context).length = 0;
         }
 
-        // FIXME: Memory Tag
         (*queue_context).buffer =
-            ExAllocatePool2(POOL_FLAG_NON_PAGED, length as SIZE_T, 's' as u32);
+            ExAllocatePool2(POOL_FLAG_NON_PAGED, length as SIZE_T, MEMORY_TAG);
         if (*queue_context).buffer.is_null() {
             println!(
                 "echo_evt_io_write Could not allocate {:?} byte buffer",
@@ -697,9 +712,8 @@ unsafe extern "C" fn echo_evt_timer_func(timer: WDFTIMER) {
             // currently racing with it), there is no need to use an interlocked
             // decrement to lower the cancel ownership count.
 
-            // 2 is the initial count we set when we initialized
-            // CancelCompletionOwnershipCount plus the call to
-            // EchoIncrementRequestCancelOwnershipCount()
+            // 2 = the initial ownership count (1) plus the one increment
+            // acquired via echo_increment_request_cancel_ownership_count.
             (*request_context)
                 .cancel_completion_ownership_count
                 .fetch_sub(2, Ordering::SeqCst);

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

@@ -72,6 +72,11 @@ static NUM_ASYNCH_IO: usize = 100;
 static BUFFER_SIZE: usize = 40 * 1024;
 
 fn main() -> Result<(), Box<dyn Error>> {
+    /// Transfer length, in bytes, for the first synchronous write/read test.
+    const SYNC_TEST_SMALL_LENGTH: u32 = 512;
+    /// Transfer length, in bytes, for the second synchronous write/read test.
+    const SYNC_TEST_LARGE_LENGTH: u32 = 30 * 1024;
+
     let argument_vector: Vec<String> = env::args().collect();
     let argument_count = argument_vector.len();
 
@@ -155,9 +160,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, SYNC_TEST_SMALL_LENGTH)?;
 
-        perform_write_read_test(h_device, 30 * 1024)?;
+        perform_write_read_test(h_device, SYNC_TEST_LARGE_LENGTH)?;
     }
 
     Ok(())
@@ -292,6 +297,14 @@ fn async_io(thread_parameter: u32) -> Result<(), Box<dyn Error + Send + Sync>> {
 // function warning
 #[allow(clippy::too_many_lines)]
 fn async_io_work(io_type: u32) -> Result<(), Box<dyn Error>> {
+    /// Completion key associated with the device handle on the I/O completion
+    /// port.
+    const COMPLETION_PORT_KEY: usize = 1;
+    /// Number of concurrent threads allowed to run for the I/O completion port.
+    /// Zero lets the system allow as many concurrent threads as there are
+    /// processors.
+    const COMPLETION_PORT_CONCURRENT_THREADS: u32 = 0;
+
     let globals = GLOBAL_DATA.read()?;
 
     let h_device: HANDLE;
@@ -334,7 +347,12 @@ fn async_io_work(io_type: u32) -> Result<(), Box<dyn Error>> {
     // Call Win32 API FFI CreateIoCompletionPort to get handle for completing async
     // requests
     unsafe {
-        h_completion_port = CreateIoCompletionPort(h_device, std::ptr::null_mut(), 1, 0);
+        h_completion_port = CreateIoCompletionPort(
+            h_device,
+            std::ptr::null_mut(),
+            COMPLETION_PORT_KEY,
+            COMPLETION_PORT_CONCURRENT_THREADS,
+        );
     }
 
     if h_completion_port.is_null() {