From cd72f77a3c653b61223ec170e7d88ca80213d90a Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 11 Aug 2026 16:28:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(pf-encode):=20the=20AMF=20layout=20guards?= =?UTF-8?q?=20broke=20Windows=20clippy=20=E2=80=94=200*SLOT=20and=201*SLOT?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `27f08340` wrote every vtable offset assertion as `offset_of!(T, f) == N * SLOT` so the slot INDEX stays visible in the assertion. For N=0 and N=1 that is `0 * SLOT` and `1 * SLOT`, which clippy rejects as `erasing_op` and `identity_op` โ€” six errors, and windows-host.yml runs clippy with `-D warnings`, so the branch as pushed would have turned the Windows leg red. This is the blind spot the programme document names in ยง1.5, demonstrated on the programme's own first code commit: 44% of the host's unsafe is `#[cfg(windows)]`, no Linux or macOS check compiles it, and `cargo fmt`/`cargo check` on a Mac are all clean. Only the .133 gate sees it. Fixed with a `const fn slot(i: usize) -> usize` rather than by writing the two offending cases as bare `0` and `SLOT`: that would have made those two the only assertions where the slot index is invisible, and the index is the entire point. Also records the cheap local gate that would have caught this without a Windows round-trip: `amf_sys.rs` depends on nothing but `c_void`, so copying it into a throwaway one-file crate and running `cargo clippy -- -D warnings` reproduces the exact error on any host. Verified by reintroducing `0 * SLOT` and watching the harness fail with the same message the runner gave. Verified on 192.168.1.133 (Windows CI runner, the box with the WDK), after a `cargo clean -p pf-encode` that reported `Removed 47 files, 135.5MiB` so the recompile is real and not a cached green: cargo check -p pf-encode ok cargo check -p pf-encode --all-targets --features nvenc,amf-qsv,qsv ok cargo clippy -p pf-encode --all-targets --features nvenc,amf-qsv,qsv -- -D warnings exit 0 (was 101) cargo clippy -p punktfunk-host --features nvenc,amf-qsv,qsv -- -D warnings exit 0 (was 101) The gate also greps the extracted tree for the assertions before building, so a stale upload cannot produce a passing run. --- crates/pf-encode/src/enc/windows/amf_sys.rs | 64 ++++++++++++--------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/crates/pf-encode/src/enc/windows/amf_sys.rs b/crates/pf-encode/src/enc/windows/amf_sys.rs index becaf0cc..04c60c9c 100644 --- a/crates/pf-encode/src/enc/windows/amf_sys.rs +++ b/crates/pf-encode/src/enc/windows/amf_sys.rs @@ -439,6 +439,14 @@ pub struct AmfBufferVtbl { /// always `index * SLOT`. const SLOT: usize = core::mem::size_of::(); +/// Byte offset of vtable slot `i`. A `const fn` rather than a bare `i * SLOT` expression because +/// clippy's `erasing_op`/`identity_op` reject `0 * SLOT` and `1 * SLOT` under the `-D warnings` +/// the Windows CI leg runs with โ€” and writing those two as bare `0` and `SLOT` would be the one +/// place the slot INDEX stops being visible, which is the entire readability of these assertions. +const fn slot(i: usize) -> usize { + i * SLOT +} + // Every slot is a plain code pointer, so all five tables are pointer-sized-array-shaped. If this // ever fails, the tables are not flat arrays any more and every offset below is meaningless. const _: () = assert!(SLOT == core::mem::size_of::()); @@ -458,43 +466,43 @@ const _: () = assert!(core::mem::offset_of!(AmfHdrMetadata, max_mastering_lumina const _: () = assert!(core::mem::offset_of!(AmfHdrMetadata, max_content_light_level) == 24); // -- AMFFactory (7 slots) โ€” `create_context` 0, `create_component` 1 -- -const _: () = assert!(core::mem::size_of::() == 7 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfFactoryVtbl, create_context) == 0 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfFactoryVtbl, create_component) == 1 * SLOT); +const _: () = assert!(core::mem::size_of::() == slot(7)); +const _: () = assert!(core::mem::offset_of!(AmfFactoryVtbl, create_context) == slot(0)); +const _: () = assert!(core::mem::offset_of!(AmfFactoryVtbl, create_component) == slot(1)); // -- AMFContext (55 slots) = AMFInterface(3) + AMFPropertyStorage(10) + AMFContext(42) -- -const _: () = assert!(core::mem::size_of::() == 55 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfContextVtbl, release) == 1 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfContextVtbl, terminate) == 13 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfContextVtbl, init_dx11) == 18 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfContextVtbl, alloc_buffer) == 43 * SLOT); +const _: () = assert!(core::mem::size_of::() == slot(55)); +const _: () = assert!(core::mem::offset_of!(AmfContextVtbl, release) == slot(1)); +const _: () = assert!(core::mem::offset_of!(AmfContextVtbl, terminate) == slot(13)); +const _: () = assert!(core::mem::offset_of!(AmfContextVtbl, init_dx11) == slot(18)); +const _: () = assert!(core::mem::offset_of!(AmfContextVtbl, alloc_buffer) == slot(43)); const _: () = - assert!(core::mem::offset_of!(AmfContextVtbl, create_surface_from_dx11_native) == 49 * SLOT); + assert!(core::mem::offset_of!(AmfContextVtbl, create_surface_from_dx11_native) == slot(49)); // -- AMFComponent (28 slots) = AMFInterface(3) + PropertyStorage(10) + StorageEx(4) + Component(11) -- -const _: () = assert!(core::mem::size_of::() == 28 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, release) == 1 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, set_property) == 3 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, init) == 17 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, terminate) == 19 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, drain) == 20 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, flush) == 21 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, submit_input) == 22 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, query_output) == 23 * SLOT); +const _: () = assert!(core::mem::size_of::() == slot(28)); +const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, release) == slot(1)); +const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, set_property) == slot(3)); +const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, init) == slot(17)); +const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, terminate) == slot(19)); +const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, drain) == slot(20)); +const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, flush) == slot(21)); +const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, submit_input) == slot(22)); +const _: () = assert!(core::mem::offset_of!(AmfComponentVtbl, query_output) == slot(23)); // -- AMFData (23 slots) = AMFInterface(3) + AMFPropertyStorage(10) + AMFData(10) -- -const _: () = assert!(core::mem::size_of::() == 23 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, release) == 1 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, query_interface) == 2 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, set_property) == 3 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, get_property) == 4 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, set_pts) == 19 * SLOT); +const _: () = assert!(core::mem::size_of::() == slot(23)); +const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, release) == slot(1)); +const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, query_interface) == slot(2)); +const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, set_property) == slot(3)); +const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, get_property) == slot(4)); +const _: () = assert!(core::mem::offset_of!(AmfDataVtbl, set_pts) == slot(19)); // -- AMFBuffer (28 slots) = the AMFData prefix (23) + AMFBuffer(5) -- -const _: () = assert!(core::mem::size_of::() == 28 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfBufferVtbl, release) == 1 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfBufferVtbl, get_size) == 24 * SLOT); -const _: () = assert!(core::mem::offset_of!(AmfBufferVtbl, get_native) == 25 * SLOT); +const _: () = assert!(core::mem::size_of::() == slot(28)); +const _: () = assert!(core::mem::offset_of!(AmfBufferVtbl, release) == slot(1)); +const _: () = assert!(core::mem::offset_of!(AmfBufferVtbl, get_size) == slot(24)); +const _: () = assert!(core::mem::offset_of!(AmfBufferVtbl, get_native) == slot(25)); // -- The shared-prefix agreement -- // `AMFBuffer` derives from `AMFData`, and `create_surface_from_dx11_native` hands back an