From 37a42078a06f2f8b34fd91ccff7b1245c6d456e0 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 28 Jul 2026 22:17:43 +0200 Subject: [PATCH] =?UTF-8?q?fix(xcheck):=20mirror=20[workspace.lints]=20too?= =?UTF-8?q?=20=E2=80=94=20and=20the=20two=20things=20that=20hid=20behind?= =?UTF-8?q?=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scripts/xcheck.sh` stopped working the moment the workspace adopted its unsafe discipline (`52191071`). That commit added `[lints] workspace = true` to every crate manifest, and a member inheriting a lint table the workspace ROOT does not define does not merely lose the lint — cargo refuses to parse the manifest at all: error inheriting `lints` from workspace root manifest's `workspace.lints` `workspace.lints` was not defined The generated root mirrored `[workspace.package]` but nothing else. It now mirrors every `[workspace.lints.*]` table generically, so a future table (clippy, rustdoc, …) is picked up without touching the script again. That mattered more than a broken helper, because xcheck is the ONLY thing that lints these crates for Windows — the Windows CI job clippies `-p punktfunk-host -p punktfunk-tray`, which does not lint a dependency. So while it was down, two real failures sat on main unseen: * **pf-frame `dxgi.rs`, 8 × `clippy::undocumented_unsafe_blocks`.** `307ca88b`/`0a710fdf` split the D3DKMT/DXGI calls into one `unsafe` block per FFI call under a single shared SAFETY comment, but the lint wants a proof per block — a comment two lines up with an intervening statement does not count. Each site now carries its own. Not "as above", which this sweep already found to be a proof that rots when the block it points at moves. * **pf-win-display `input_desktop.rs`, one E0133.** `GetUserObjectInformationW` is an unsafe operation in an `unsafe fn` body, which `unsafe_op_in_unsafe_fn` (now warn workspace-wide, and `-D warnings` here) requires to sit in its own block. The SAFETY proof was already written; only the block was missing. The `&&` chain in `elevate_process_gpu_priority` keeps two separate blocks rather than one around the chain — wrapping it would destroy the short-circuit and always issue the relative-priority call. Same trap this sweep hit in `wait_mode_settled`. Verified: both xcheck targets clean again. --- crates/pf-frame/src/dxgi.rs | 16 ++++++++++++++++ crates/pf-win-display/src/input_desktop.rs | 16 +++++++++------- scripts/xcheck.sh | 8 ++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/crates/pf-frame/src/dxgi.rs b/crates/pf-frame/src/dxgi.rs index f4e97e71..609e1312 100644 --- a/crates/pf-frame/src/dxgi.rs +++ b/crates/pf-frame/src/dxgi.rs @@ -134,6 +134,9 @@ pub unsafe fn make_device(adapter: &IDXGIAdapter1) -> Result<(ID3D11Device, ID3D // SAFETY: `dxgi_dev` is a live interface just obtained by a checked `cast`; both calls take // a scalar and only report failure through their return value. if unsafe { dxgi_dev.SetGPUThreadPriority(0x4000_001E) }.is_err() + // SAFETY: same live interface, same scalar-in/HRESULT-out contract as the call above. + // Deliberately its own block rather than one around the whole chain, which would + // destroy the short-circuit and always issue the relative-priority call too. && unsafe { dxgi_dev.SetGPUThreadPriority(7) }.is_err() { tracing::warn!("SetGPUThreadPriority failed (run as admin/SYSTEM for GPU priority)"); @@ -256,12 +259,17 @@ unsafe fn d3dkmt_set_scheduling_priority_class( // process keeps for its lifetime (gdi32 is never unloaded here), and `GetProcAddress` is passed // that live handle. Both results are checked by `?` before use. let gdi32 = unsafe { LoadLibraryA(s!("gdi32.dll")) }.ok()?; + // SAFETY: `gdi32` is the live module handle the checked `LoadLibraryA` just returned, and the + // export name is a static NUL-terminated literal; the result is checked by `?` before use. let p = unsafe { GetProcAddress(gdi32, s!("D3DKMTSetProcessSchedulingPriorityClass")) }?; type SetPrio = unsafe extern "system" fn(HANDLE, i32) -> i32; // SAFETY: `p` is the non-null export just resolved, and `SetPrio` is its documented signature // (`NTSTATUS D3DKMTSetProcessSchedulingPriorityClass(HANDLE, D3DKMT_SCHEDULINGPRIORITYCLASS)`, // both arguments 4/8-byte scalars). `process` is a valid handle by this fn's own contract. let f: SetPrio = unsafe { std::mem::transmute(p) }; + // SAFETY: `f` is that export transmuted to its documented signature directly above; `process` + // is a valid handle by this fn's own contract and `prio` is a plain scalar. The call returns an + // NTSTATUS and retains nothing. Some(unsafe { f(process, prio) }) } @@ -365,8 +373,12 @@ fn hags_enabled(luid: LUID) -> Option { // SAFETY: static NUL-terminated literals; gdi32 stays loaded for the process lifetime, and each // result is checked by `?`/`.ok()?` before the next call uses it. let gdi32 = unsafe { LoadLibraryA(s!("gdi32.dll")) }.ok()?; + // SAFETY: `gdi32` is the live handle from the `.ok()?`-checked load above; static literal name; + // `?` checks the result. let open = unsafe { GetProcAddress(gdi32, s!("D3DKMTOpenAdapterFromLuid")) }?; + // SAFETY: same live `gdi32` handle and static-literal name; `?` checks the result. let query = unsafe { GetProcAddress(gdi32, s!("D3DKMTQueryAdapterInfo")) }?; + // SAFETY: same live `gdi32` handle and static-literal name; `?` checks the result. let close = unsafe { GetProcAddress(gdi32, s!("D3DKMTCloseAdapter")) }?; type OpenFn = unsafe extern "system" fn(*mut OpenFromLuid) -> i32; type QueryFn = unsafe extern "system" fn(*mut QueryInfo) -> i32; @@ -375,7 +387,11 @@ fn hags_enabled(luid: LUID) -> Option { // export's documented signature — one `*mut` to the matching `repr(C)` struct declared here, // returning NTSTATUS. let open: OpenFn = unsafe { std::mem::transmute(open) }; + // SAFETY: `query` is the non-null export resolved above and `QueryFn` mirrors its documented + // signature — one `*mut` to the `repr(C)` struct declared here, returning NTSTATUS. let query: QueryFn = unsafe { std::mem::transmute(query) }; + // SAFETY: `close` is the non-null export resolved above and `CloseFn` mirrors its documented + // signature — one `*mut` to the `repr(C)` struct declared here, returning NTSTATUS. let close: CloseFn = unsafe { std::mem::transmute(close) }; let mut oa = OpenFromLuid { luid, h_adapter: 0 }; diff --git a/crates/pf-win-display/src/input_desktop.rs b/crates/pf-win-display/src/input_desktop.rs index 4eab9ea7..f4acba7a 100644 --- a/crates/pf-win-display/src/input_desktop.rs +++ b/crates/pf-win-display/src/input_desktop.rs @@ -133,13 +133,15 @@ unsafe fn desktop_name(h: HDESK) -> Option { let mut needed = 0u32; // SAFETY: `h` is live per this fn's contract; `name`/`needed` are live out-params and the call // writes at most `nlength` bytes, exactly the size passed. - GetUserObjectInformationW( - HANDLE(h.0), - UOI_NAME, - Some(name.as_mut_ptr().cast()), - (name.len() * 2) as u32, - Some(&mut needed), - ) + unsafe { + GetUserObjectInformationW( + HANDLE(h.0), + UOI_NAME, + Some(name.as_mut_ptr().cast()), + (name.len() * 2) as u32, + Some(&mut needed), + ) + } .ok()?; let len = name.iter().position(|&c| c == 0).unwrap_or(name.len()); Some(String::from_utf16_lossy(&name[..len])) diff --git a/scripts/xcheck.sh b/scripts/xcheck.sh index ee3b2a67..1b6e4dbd 100755 --- a/scripts/xcheck.sh +++ b/scripts/xcheck.sh @@ -111,6 +111,14 @@ mkdir -p "$OUT" echo echo '[workspace.package]' sed -n '/^\[workspace\.package\]/,/^$/p' "$REPO/Cargo.toml" | sed '1d;/^$/d' + # …and the real [workspace.lints.*] tables. Every crate manifest now carries + # `[lints] workspace = true` (the workspace-wide unsafe discipline), and a member inheriting a + # lint table the generated ROOT does not define does not merely lose the lint — cargo refuses to + # parse the manifest at all ("error inheriting `lints` from workspace root manifest's + # `workspace.lints`"), which broke this script outright the day that landed. Mirrored generically + # so a new table (clippy, rustdoc, …) is picked up without touching this script again. + echo + awk '/^\[/ { in_lints = ($0 ~ /^\[workspace\.lints/) } in_lints' "$REPO/Cargo.toml" } > "$OUT/Cargo.toml" mkdir -p "$OUT/punktfunk-core/src"