refactor(windows): migrate from winapi to windows crate
#47
Conversation
WalkthroughThe code updates Windows threading support by migrating from the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ThreadingAPI
participant WindowsAPI
Caller->>ThreadingAPI: set_winapi_thread_priority(ThreadId, WinAPIThreadPriority)
ThreadingAPI->>WindowsAPI: SetThreadPriority(ThreadId.0, WinAPIThreadPriority.0)
WindowsAPI-->>ThreadingAPI: Result
ThreadingAPI-->>Caller: Result<(), Error>
sequenceDiagram
participant Caller
participant ThreadingAPI
participant WindowsAPI
Caller->>ThreadingAPI: get_thread_priority(ThreadId)
ThreadingAPI->>WindowsAPI: GetThreadPriority(ThreadId.0)
WindowsAPI-->>ThreadingAPI: THREAD_PRIORITY
ThreadingAPI-->>Caller: ThreadPriority or Error
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/windows.rs (1)
90-106: 🛠️ Refactor suggestionGaps in cross-platform→Windows priority mapping (20, 40, 60, 80).
Values
20,40,60, and80currently fall through the match and raiseError::Priority.
Unless this is deliberate, enlarge the affected ranges so that the entire[0;99]space is accepted.-1..=19 => WinAPIThreadPriority::Lowest, -21..=39 => WinAPIThreadPriority::BelowNormal, -41..=59 => WinAPIThreadPriority::Normal, -61..=79 => WinAPIThreadPriority::AboveNormal, -81..=98 => WinAPIThreadPriority::Highest, +1..=20 => WinAPIThreadPriority::Lowest, +21..=40 => WinAPIThreadPriority::BelowNormal, +41..=60 => WinAPIThreadPriority::Normal, +61..=80 => WinAPIThreadPriority::AboveNormal, +81..=98 => WinAPIThreadPriority::Highest,
🧹 Nitpick comments (3)
src/windows.rs (3)
25-27: Mark newtype as#[repr(transparent)]for FFI correctness.
IdealProcessoris passed verbatim to the Windows API, so its memory layout must be identical tou32.
Annotating with#[repr(transparent)]makes that guarantee explicit and future-proof.-#[derive(Clone, Debug, Default, Hash, Eq, PartialEq, Ord, PartialOrd)] -pub struct IdealProcessor(u32); +#[repr(transparent)] +#[derive(Clone, Debug, Default, Hash, Eq, PartialEq, Ord, PartialOrd)] +pub struct IdealProcessor(u32);
29-33: Consider derivingHash&Ordinstead of manual impls.
ThreadIdalready defines a customOrd, but the same ordering (by raw handle value) can be obtained automatically with#[derive(Ord, PartialOrd, Hash)]becauseHANDLEisCopy + Ord.
This removes unsafe casting and reduces maintenance.If you keep the manual impl, document that ordering by raw address is intentional.
358-375: Duplicated constant list – consider factoring into a helper.The same exhaustive match on
THREAD_PRIORITYvalues appears twice (TryFrom<ThreadPriority>and here).
Moving the mapping to a singlefn map_priority(i32) -> Option<WinAPIThreadPriority>avoids drift and simplifies maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
Cargo.tomlis excluded by!**/*.toml
📒 Files selected for processing (2)
src/lib.rs(1 hunks)src/windows.rs(12 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/windows.rs (2)
src/unix.rs (2)
try_from(847-853)thread_native_id(840-842)src/lib.rs (2)
try_from(257-267)ideal_processor(557-560)
🔇 Additional comments (1)
src/lib.rs (1)
279-282: Breaking-change alert: inner type switched fromu32→i32.
ThreadPriorityOsValuenow wraps ani32. This is source-level breaking for anyone pattern-matching or constructing the tuple struct directly.
Please double-check:
- Public re-exports /
pub usepaths that expose the oldu32still compile.- All
TryFrom/Intoimpls in downstream modules (e.g.unix, tests, docs) compile and behave identically for negative values.If this crate is ≥
1.0, remember to bump themajorversion or provide a deprecation shim.
|
Wow, thank you so much! I will review it later this week. If I suddenly forget, please ping me! This is certainly useful and a good change to this crate, I'd even call it necessary. |
| if ret == IdealProcessor::max_value() - 1 { | ||
| Err(Error::OS(GetLastError() as i32)) | ||
| let ret = SetThreadIdealProcessor(native.0, ideal_processor); | ||
| if ret == u32::MAX { |
There was a problem hiding this comment.
Minor bugfix: (DWORD)-1 is u32::MAX.
https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-setthreadidealprocessor#return-value
|
ping @iddm |
|
Hey, thanks again! Would you fix the clippy lints, or would you rather want me to do that? |
Hi, #48 was created to resolve these warnings. |
…read priority handling This commit updates the codebase to use the `windows` crate instead of the deprecated `winapi` crate. Key changes include: - Replaced `winapi` imports with `windows` crate equivalents. - Refactored thread priority handling to use the new `windows` crate enums and functions. - Improved type safety and readability by introducing new types for `IdealProcessor` and `ThreadId`.
|
Thanks once again! |
2 participants
This commit updates the codebase to use the
windowscrate instead of the deprecatedwinapicrate. Key changes include:winapiimports withwindowscrate equivalents.ThreadPriorityOsValuefromu32toi32to align with the new crate's types.windowscrate enums and functions.IdealProcessorandThreadId.