diff --git a/crates/pf-vdisplay/src/lib.rs b/crates/pf-vdisplay/src/lib.rs index ff0d9557..ed5abf5c 100644 --- a/crates/pf-vdisplay/src/lib.rs +++ b/crates/pf-vdisplay/src/lib.rs @@ -17,6 +17,11 @@ #![allow(dead_code)] // Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). #![deny(clippy::undocumented_unsafe_blocks)] +// …and that program only covers a whole `unsafe fn` body once the body needs its own block: in +// edition 2021 `unsafe_op_in_unsafe_fn` is allow-by-default, which exempted this crate's hardest +// FFI from the deny above — every IOCTL wrapper, and `restore_displays_ccd`, the call the whole +// Windows teardown path depends on to give the operator their physical panels back. +#![deny(unsafe_op_in_unsafe_fn)] use anyhow::Result; pub use punktfunk_core::Mode; diff --git a/crates/pf-vdisplay/src/vdisplay/windows/manager.rs b/crates/pf-vdisplay/src/vdisplay/windows/manager.rs index 70ed2617..428edb64 100644 --- a/crates/pf-vdisplay/src/vdisplay/windows/manager.rs +++ b/crates/pf-vdisplay/src/vdisplay/windows/manager.rs @@ -1594,7 +1594,14 @@ impl VirtualDisplayManager { // displays. inner.group.ccd_exclusive = false; if let Some(saved) = inner.group.ccd_saved.take() { - restore_displays_ccd(&saved); + // SAFETY: `saved` is a `SavedConfig` this manager captured from + // `isolate_displays_ccd`/`set_virtual_primary_ccd` on this same box (it can be + // produced no other way), so its path+mode arrays are a self-consistent CCD + // topology and are passed with their own lengths. `restore_displays_ccd` binds + // itself to the input desktop internally (`retry_set_display_config`), which is + // the one thing its callers could otherwise get wrong. The `take()` above makes + // this the only consumer of that snapshot, so no second restore can race it. + unsafe { restore_displays_ccd(&saved) }; // EXPERIMENTAL `ddc_power_off` wake: the restore re-activated the physical paths, and // returning signal alone wakes DPMS-off panels on most firmware — the explicit ON is // belt-and-braces for the rest. The brief settle wait lets the re-activated paths show diff --git a/crates/pf-vdisplay/src/vdisplay/windows/pf_vdisplay.rs b/crates/pf-vdisplay/src/vdisplay/windows/pf_vdisplay.rs index a32fccbd..36ae1e04 100644 --- a/crates/pf-vdisplay/src/vdisplay/windows/pf_vdisplay.rs +++ b/crates/pf-vdisplay/src/vdisplay/windows/pf_vdisplay.rs @@ -57,20 +57,36 @@ fn next_session_id() -> u64 { /// One `DeviceIoControl` round trip (METHOD_BUFFERED). `input`/`output` may be empty. Identical to the /// SudoVDA backend's wrapper; struct<->bytes conversion happens at the call sites via `bytemuck`. +/// +/// # Safety +/// +/// `h` must be a live handle to the pf-vdisplay control device — one returned by [`open_device`] +/// and not yet closed. Every other obligation is discharged inside: the two buffer pointers are +/// derived from the caller's slices and are passed with exactly those slices' lengths, and the +/// slices outlive the call. unsafe fn ioctl(h: HANDLE, code: u32, input: &[u8], output: &mut [u8]) -> Result { let mut returned = 0u32; let inp = (!input.is_empty()).then_some(input.as_ptr() as *const c_void); let outp = (!output.is_empty()).then_some(output.as_mut_ptr() as *mut c_void); - DeviceIoControl( - h, - code, - inp, - input.len() as u32, - outp, - output.len() as u32, - Some(&mut returned), - None, - ) + // SAFETY: `h` is a live control-device handle by this fn's contract. `inp`/`outp` are derived + // from `input`/`output` and paired with those slices' own lengths, so the kernel reads exactly + // `input.len()` initialised bytes and writes at most `output.len()` bytes it is entitled to; + // both slices are borrowed for the whole call. `Some(&mut returned)` is a live local. This is + // METHOD_BUFFERED, so the kernel copies through its own system buffer rather than retaining + // either pointer, and `None` for the OVERLAPPED makes the call synchronous — nothing outlives + // the call to alias. + unsafe { + DeviceIoControl( + h, + code, + inp, + input.len() as u32, + outp, + output.len() as u32, + Some(&mut returned), + None, + ) + } .with_context(|| format!("DeviceIoControl(code={code:#x})"))?; Ok(returned) } @@ -194,18 +210,29 @@ fn is_slot_exhaustion_wedge(e: &anyhow::Error) -> bool { /// different adapter than the one we duplicate/encode on (the ACCESS_LOST storm). The driver /// implements it (`control.rs` → `adapter::set_render_adapter`); callers still tolerate an `Err` /// (warn + continue) since the driver reports its real render LUID in the shared header either way. +/// +/// # Safety +/// +/// `h` must be a live handle to the pf-vdisplay control device — [`ioctl`]'s obligation, and the +/// only one. `luid` is plain `Copy` data with no validity requirement of its own. unsafe fn set_render_adapter(h: HANDLE, luid: LUID) -> Result<()> { let req = control::SetRenderAdapterRequest { luid_low: luid.LowPart, luid_high: luid.HighPart, }; let mut none: [u8; 0] = []; - ioctl( - h, - control::IOCTL_SET_RENDER_ADAPTER, - bytemuck::bytes_of(&req), - &mut none, - ) + // SAFETY: `h` is a live control-device handle by this fn's contract — the one thing `ioctl` + // asks of its caller. The request is a `Pod` struct viewed through `bytemuck::bytes_of`, so + // the input slice is exactly its initialised bytes, and the empty output slice matches the + // IOCTL's "no output buffer" contract. + unsafe { + ioctl( + h, + control::IOCTL_SET_RENDER_ADAPTER, + bytemuck::bytes_of(&req), + &mut none, + ) + } .map(|_| ()) .context("pf-vdisplay SET_RENDER_ADAPTER") } diff --git a/scripts/wincheck.sh b/scripts/wincheck.sh index d2fc5f85..cc7b6e5b 100755 --- a/scripts/wincheck.sh +++ b/scripts/wincheck.sh @@ -21,13 +21,19 @@ # Both enter the graph through `punktfunk-core`'s `quic` feature. Stubbing opus with a fake # `opus.pc` gets past audiopus but not ring. # -# The whole Windows capture stack uses exactly TWO items from punktfunk-core — `quic::HdrMeta` and -# `Mode`, both plain data (re-verify with: -# `grep -rho 'punktfunk_core::[A-Za-z_:]*' crates/pf-capture/src crates/pf-frame/src crates/pf-win-display/src | sort -u`). -# So this script generates a ~30-line stub core carrying just those, and rustls/ring/opus never +# The whole Windows capture stack uses exactly FOUR items from punktfunk-core — `quic::HdrMeta`, +# `Mode`, `CompositorPref` and its `as_str`, all plain data (re-verify with: +# `grep -rho 'punktfunk_core::[A-Za-z_:]*' crates/pf-{capture,frame,win-display,vdisplay}/src | sort -u`). +# So this script generates a ~50-line stub core carrying just those, and rustls/ring/opus never # enter the graph at all. Every path dep that is NOT a generated member points at the REAL crate by # absolute path, so their own relative deps and `version.workspace` inheritance still resolve. # +# pf-encode is stubbed for the same reason and needs the same care: pf-vdisplay's admission gate +# calls exactly ONE item from it (`can_open_another_session`, admission.rs), but the real crate +# drags ffmpeg-sys — whose build script wants a Windows FFmpeg tree this box does not have. The +# stub is a `cfg(windows) -> bool`; if admission ever consults a second encoder fact, mirror it +# here or the cross-check stops covering that call. +# # Note: `cargo fmt` needs none of this — rustfmt follows `mod`/`#[path]` without evaluating cfg, so # it already reaches Windows-only files. Mechanical edits can be normalised with plain `cargo fmt`. set -euo pipefail @@ -40,8 +46,8 @@ TARGET=x86_64-pc-windows-msvc # dep naming one of these must stay RELATIVE (resolving to the generated member); anything else is # rewritten to the real crate. Getting that backwards silently drags the real punktfunk-core — and # with it ring and opus — back in through pf-frame, which is the entire thing this avoids. -MEMBERS_RE='punktfunk-core|pf-frame|pf-win-display|pf-capture' -REAL_MEMBERS=(pf-frame pf-win-display pf-capture) +MEMBERS_RE='punktfunk-core|pf-encode|pf-frame|pf-win-display|pf-capture|pf-vdisplay' +REAL_MEMBERS=(pf-frame pf-win-display pf-capture pf-vdisplay) cmd="${1:-clippy}" [ $# -gt 0 ] && shift @@ -71,7 +77,7 @@ mkdir -p "$OUT" echo '# GENERATED by scripts/wincheck.sh — do not edit; re-run the script.' echo '[workspace]' echo 'resolver = "2"' - echo 'members = ["punktfunk-core", "pf-frame", "pf-win-display", "pf-capture"]' + echo 'members = ["punktfunk-core", "pf-encode", "pf-frame", "pf-win-display", "pf-capture", "pf-vdisplay"]' echo echo '[workspace.package]' sed -n '/^\[workspace\.package\]/,/^$/p' "$REPO/Cargo.toml" | sed '1d;/^$/d' @@ -108,6 +114,30 @@ pub struct Mode { pub refresh_hz: u32, } +/// Mirrors `punktfunk_core::CompositorPref` (config.rs). pf-vdisplay matches on every variant and +/// calls `as_str`, so both the variant set and the strings must stay in step with the real enum. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub enum CompositorPref { + #[default] + Auto, + Kwin, + Wlroots, + Mutter, + Gamescope, +} + +impl CompositorPref { + pub fn as_str(self) -> &'static str { + match self { + CompositorPref::Auto => "auto", + CompositorPref::Kwin => "kwin", + CompositorPref::Wlroots => "wlroots", + CompositorPref::Mutter => "mutter", + CompositorPref::Gamescope => "gamescope", + } + } +} + pub mod quic { /// Mirrors `punktfunk_core::quic::HdrMeta`. #[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] @@ -122,6 +152,28 @@ pub mod quic { } EOF +mkdir -p "$OUT/pf-encode/src" +cat > "$OUT/pf-encode/Cargo.toml" <<'EOF' +# GENERATED by scripts/wincheck.sh — a STUB, not the real crate. +[package] +name = "pf-encode" +version.workspace = true +edition = "2021" +rust-version.workspace = true +EOF + +cat > "$OUT/pf-encode/src/lib.rs" <<'EOF' +//! GENERATED by scripts/wincheck.sh — a STUB of pf-encode carrying only the one item pf-vdisplay's +//! admission gate uses. The real crate pulls ffmpeg-sys, whose build script needs a Windows FFmpeg +//! tree; nothing in the display path needs the encoder itself, only this predicate. + +/// Mirrors `pf_encode::can_open_another_session` (lib.rs, `#[cfg(target_os = "windows")]`). +#[cfg(target_os = "windows")] +pub fn can_open_another_session() -> bool { + true +} +EOF + for name in "${REAL_MEMBERS[@]}"; do mkdir -p "$OUT/$name" # Park the member paths behind a sentinel first: a plain self-substitution would be a no-op and