refactor: remove magic numbers#64
Conversation
|
|
||
| /// 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"); |
There was a problem hiding this comment.
This doesn't match the existing tag
There was a problem hiding this comment.
I noticed that it was different from the original C echo sample driver so I changed it to match the C version.
| /// Transfer length, in bytes, for the first synchronous write/read test. | ||
| static SYNC_TEST_SMALL_LENGTH: u32 = 512; | ||
| /// Transfer length, in bytes, for the second synchronous write/read test. | ||
| static SYNC_TEST_LARGE_LENGTH: u32 = 30 * 1024; | ||
| /// Completion key associated with the device handle on the I/O completion port. | ||
| static 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. | ||
| static COMPLETION_PORT_CONCURRENT_THREADS: u32 = 0; |
There was a problem hiding this comment.
why are these static instead of const?
There was a problem hiding this comment.
Good catch, I was just following the semantics of the previous static variable definitions above them.
| /// Number of 100-nanosecond intervals in one millisecond. WDF relative times | ||
| /// are expressed in 100-nanosecond units, so this converts a millisecond delay | ||
| /// into the units WDF expects. | ||
| const RELATIVE_100_NS_INTERVALS_PER_MS: i64 = 10_000; | ||
|
|
||
| /// Delay, in milliseconds, before the periodic timer first fires once the | ||
| /// device has started. | ||
| const START_TIMER_DUE_TIME_MS: i64 = 100; | ||
|
|
||
| /// 100ms relative time (in 100-nanosecond units). The negative sign marks the | ||
| /// value as a relative (rather than absolute) timeout. | ||
| const WDF_REL_TIMEOUT_IN_MS: i64 = -START_TIMER_DUE_TIME_MS * RELATIVE_100_NS_INTERVALS_PER_MS; | ||
|
|
There was a problem hiding this comment.
rather than introduce more constants here, is it possible to just use the conversion function in the rust standard library: https://doc.rust-lang.org/core/time/struct.Duration.html
they are available in core
There was a problem hiding this comment.
Fixed, I changed the extra timer constants to utilize Duration, it should help with readability now.
| /// Set timer period in ms | ||
| const TIMER_PERIOD: u32 = 1000 * 10; | ||
| /// Max write length, in bytes, for testing | ||
| const MAX_WRITE_LENGTH: usize = 40 * BYTES_PER_KB; |
There was a problem hiding this comment.
unless these are actually being used in multiple places, it'd be nice to scope them locally to where they are needed
There was a problem hiding this comment.
Fixed. I applied this to the other constants as well.
…s into function scope as appropriate,
2 participants
Summary
Decompose magic numbers into
const's adding relevant documentation.