fabi/Sylpheed
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(formats): clear all 43 clippy lints in sylpheed-formats

Browse Source 
Run 204 gave this repository its first clippy measurement — 48 errors, 43
of them in `sylpheed-formats`. This clears that 43 to zero under the exact
invocation CI runs, `cargo clippy -p sylpheed-formats -- -D warnings`.

Why this crate first, and why it is safe to touch:

Every one of the 43 sites was checked against the line ranges that
`auto/frame-blend-draw-path` (495 commits) and `auto/port-p6-audio` (366)
actually modify. None of them overlap. Eleven of the fifteen affected
files are byte-identical on both branches, including `mesh.rs` and
`texture.rs`, which carry 28 of the hits between them. The three sites in
`audio.rs`, `ui_layout.rs` and `slb.rs` that live in files those branches
do change fall outside every modified hunk. The collision argument that
defers #12 does not transfer here; it was tested rather than assumed.

It also unblocks a measurement. `-D warnings` turns a lint in this crate
into a hard compile error, so its dependents never build — `sylpheed-cli`
and `sylpheed-viewer` have never been linted at all, and viewer is the
largest crate in the workspace. Both depend only on `sylpheed-formats`
(`sylpheed-export` pins it from a git tag instead), so this commit is what
makes their real counts knowable.

  38  applied by `cargo clippy --fix` — chunks_exact_to_as_chunks,
      manual_div_ceil / is_multiple_of / range_contains, unnecessary_map_or,
      needless_borrow, let_and_return, dead_code, unused_mut/variables.
      Purely local expression rewrites: 38 insertions, 39 deletions.
   2  by hand: a doc continuation that markdown was parsing as a list, and
      `d / frame` behind a `frame > 0` guard becoming `checked_div`.
   3  `#[allow(clippy::too_many_arguments)]` with a stated reason.

On those three allows: 8 parameters against a threshold of 7, in the mesh
anchor path. The real fix is a shared params struct across
`anchor_pool_mesh`, `validate_block` and `validate_block_report` — the
latter two take the same eight arguments and one delegates to the other —
which is a change to the decoder's signatures and belongs to whoever owns
that path, not to a CI-lint pass.

This is not the shape PROTOCOL.md forbids. `continue-on-error` suppresses
everything, present and future, at the job level, and cannot tell "not
yet" from "no longer". A site-local `#[allow]` with a reason is a decision
recorded where it applies: one lint, one function, and any new violation
anywhere else still fails the build.

`sylpheed-export`'s remaining 5 are deliberately untouched — three of them
sit inside hunks both long-lived branches modify, and that crate blocks
nothing. Left for #13.

Refs #13

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01McNbzUeq1KRBWs4G6X2YVj
This commit is contained in:
sylph-decoder
2026-09-05 16:59:57 +02:00
parent d8dcc1fc29
commit bc79817488
13 changed files with 56 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
crates/sylpheed-formats/src/audio.rs
View File

@@ -226,9 +226,10 @@ fn parse_riff_wave(bytes: &[u8]) -> Option<AudioInfo> {
match codec {
AudioCodec::Pcm | AudioCodec::PcmFloat => {
if let (Some(d), true) = (data_bytes, channels > 0 && bits > 0) {
// `bits` under 8 makes the frame size zero, so this stays
// fallible: `checked_div` states that once, where it happens.
let frame = channels as u64 * (bits as u64 / 8);
if frame > 0 {
let spc = d / frame;
if let Some(spc) = d.checked_div(frame) {
info.samples_per_channel = Some(spc);
if rate > 0 {
info.duration_secs = Some(spc as f32 / rate as f32);
@@ -275,11 +276,11 @@ impl GameAudio {
let samples: Vec<f32> = match (info.codec, bits) {
(AudioCodec::Pcm, 8) => data.iter().map(|&b| (b as f32 - 128.0) / 128.0).collect(),
(AudioCodec::Pcm, 16) => data
.chunks_exact(2)
.as_chunks::<2>().0.iter()
.map(|c| i16::from_le_bytes([c[0], c[1]]) as f32 / 32768.0)
.collect(),
(AudioCodec::Pcm, 24) => data
.chunks_exact(3)
.as_chunks::<3>().0.iter()
.map(|c| {
let v = ((c[2] as i32) << 16) | ((c[1] as i32) << 8) | c[0] as i32;
let v = (v << 8) >> 8; // sign-extend 24→32
@@ -287,7 +288,7 @@ impl GameAudio {
})
.collect(),
(AudioCodec::PcmFloat, 32) => data
.chunks_exact(4)
.as_chunks::<4>().0.iter()
.map(|c| f32::from_le_bytes([c[0], c[1], c[2], c[3]]))
.collect(),
_ => return Err(AudioError::UnsupportedPcm { tag: 0, bits }),

2
crates/sylpheed-formats/src/game_data.rs
View File

@@ -1131,7 +1131,7 @@ pub fn load_squadrons(pak: &PakArchive) -> Vec<Squadron> {
let Some(f) = records.record(id) else { continue };
let slots: Vec<&str> = f.positional.iter().map(|(_, v)| v.as_str()).collect();
let members = slots
.chunks_exact(4)
.as_chunks::<4>().0.iter()
.map(|m| SquadronMember {
unit: m[0].to_string(),
message_set: text(Some(m[1])),

2
crates/sylpheed-formats/src/hash.rs
View File

@@ -71,7 +71,7 @@ pub fn name_hash(name: &str) -> u32 {
// through untouched, including any high-bit bytes, which are then sign-extended).
let mut bytes = name.as_bytes().to_vec();
for byte in &mut bytes {
if (b'A'..=b'Z').contains(byte) {
if byte.is_ascii_uppercase() {
*byte += 0x20;
}
}

2
crates/sylpheed-formats/src/ixud.rs
View File

@@ -119,7 +119,7 @@ pub fn parse(bytes: &[u8]) -> Option<Subtitle> {
fn utf16be_tokens(bytes: &[u8]) -> Vec<String> {
let mut tokens = Vec::new();
let mut cur = String::new();
for pair in bytes.chunks_exact(2) {
for pair in bytes.as_chunks::<2>().0 {
let u = u16::from_be_bytes([pair[0], pair[1]]);
match char::from_u32(u as u32) {
Some(c) if !c.is_control() => cur.push(c),

43
crates/sylpheed-formats/src/mesh.rs
View File

@@ -700,7 +700,7 @@ impl Xbg7Model {
let (vc, ic) = r.markers[0];
let sig = (r.decl.stride, vc, ic);
let floor = last_by_sig.get(&sig).map_or(0, |o| o + 1);
if m.meshes[0].vbuf_offset.map_or(false, |o| o < floor) {
if m.meshes[0].vbuf_offset.is_some_and(|o| o < floor) {
if let Some(alt) = anchor_pool_mesh(
bytes,
starts,
@@ -1339,6 +1339,13 @@ fn vertex_run_starts(bytes: &[u8], data_base: usize, stride: usize) -> Vec<usize
/// the `index_count` indices ending just before it are all `< vtx_count`,
/// reference (nearly) all vertices, and produce non-degenerate triangles with a
/// real spatial extent — a signature strong enough to pin the block.
// 8 parameters against clippy's threshold of 7. Not refactored here on
// purpose: the fix is a shared params struct across this and the two
// `validate_block*` functions below, which is a change to the decoder's
// mesh-anchoring signatures and belongs to whoever owns that path — not to
// a CI-lint pass. Recorded rather than silenced: this allow covers one lint
// at one site, and any new violation elsewhere still fails the build.
#[allow(clippy::too_many_arguments)]
fn anchor_pool_mesh(
bytes: &[u8],
starts: &[usize],
@@ -1406,7 +1413,7 @@ fn anchor_pool_mesh(
}
let (degen, wind) = index_run_quality(bytes, ib, vb, index_count, decl);
let cand = (degen, -wind, pad);
if best.map_or(true, |b| cand < b) {
if best.is_none_or(|b| cand < b) {
best = Some(cand);
}
}
@@ -1452,6 +1459,11 @@ fn anchor_pool_mesh(
/// for both the adjacency anchor ([`anchor_pool_mesh`]) and the grouped-pool
/// anchor ([`anchor_grouped_meshes`]); the two differ only in how they *place*
/// `ib`/`vb`, not in how they validate a placement.
// 8/7, as above. This and `validate_block_report` take the *same* eight
// parameters — one delegates straight to the other — which is exactly why a
// params struct is the right fix and why it should be done once, deliberately,
// rather than piecemeal here.
#[allow(clippy::too_many_arguments)]
fn validate_block(
bytes: &[u8],
ib: usize,
@@ -1478,6 +1490,8 @@ fn validate_block(
/// [`validate_block`], but naming the gate that rejected a block. A runtime
/// capture can prove a block is real; when the decoder still refuses it, this
/// says which test is wrong rather than leaving a threshold to be guessed at.
// 8/7 — the signature `validate_block` delegates into. See above.
#[allow(clippy::too_many_arguments)]
fn validate_block_report(
bytes: &[u8],
ib: usize,
@@ -1679,7 +1693,7 @@ fn anchor_grouped_meshes(
if n == 0 || decls.len() < n {
return Vec::new();
}
let decl = &decls[0];
let _decl = &decls[0];
// Relative index-buffer offsets (ib0 = 0), 4-byte aligned between buffers,
// and cumulative vertex offsets (vb0 = 0) — both derived from the marker list.
@@ -1723,7 +1737,7 @@ fn anchor_grouped_meshes(
let (vc, ic) = markers[i];
let ib = ib0 + rel_ib[i];
if ib + ic * 2 > bytes.len()
|| vc.checked_mul(decls[i].stride).map_or(true, |b| vb + b > bytes.len())
|| vc.checked_mul(decls[i].stride).is_none_or(|b| vb + b > bytes.len())
{
break;
}
@@ -1784,7 +1798,7 @@ fn anchor_grouped_meshes(
}
let (degen, wind) = index_run_quality(bytes, ib_k, vb_k, ick, &decls[kmax]);
let cand = (degen, -wind, pad);
if best.map_or(true, |b| cand < b) {
if best.is_none_or(|b| cand < b) {
best = Some(cand);
}
if pad_first_match() {
@@ -1797,7 +1811,7 @@ fn anchor_grouped_meshes(
if meshes.len() == n {
return meshes;
}
if partial.as_ref().map_or(true, |p| meshes.len() > p.len()) {
if partial.as_ref().is_none_or(|p| meshes.len() > p.len()) {
partial = Some(meshes);
}
}
@@ -1929,7 +1943,7 @@ fn find_index_marker(desc: &[u8]) -> Option<(usize, usize)> {
while rel + 40 <= desc.len() {
let a = be32(desc, rel);
let c = be32(desc, rel + 4);
if c >= 3 && c % 3 == 0 && c < 400_000 && a == c * 2 {
if c >= 3 && c.is_multiple_of(3) && c < 400_000 && a == c * 2 {
return Some((rel, c as usize));
}
rel += 4;
@@ -1951,7 +1965,7 @@ fn all_index_markers(desc: &[u8]) -> Vec<(usize, usize)> {
while rel + 8 <= desc.len() {
let a = be32(desc, rel);
let c = be32(desc, rel + 4);
if c >= 3 && c % 3 == 0 && c < 400_000 && a == c * 2 && rel >= 32 {
if c >= 3 && c.is_multiple_of(3) && c < 400_000 && a == c * 2 && rel >= 32 {
let vc = be32(desc, rel - 32) as usize;
if (3..=200_000).contains(&vc) {
out.push((vc, c as usize));
@@ -1979,7 +1993,7 @@ fn all_vertex_decls(desc: &[u8]) -> Vec<VertexDecl> {
while rel + 8 <= desc.len() {
let a = be32(desc, rel);
let c = be32(desc, rel + 4);
if c >= 3 && c % 3 == 0 && c < 400_000 && a == c * 2 && rel >= 32 {
if c >= 3 && c.is_multiple_of(3) && c < 400_000 && a == c * 2 && rel >= 32 {
let vc = be32(desc, rel - 32) as usize;
if (3..=200_000).contains(&vc) {
match parse_decl_at(desc, rel + 8) {
@@ -2086,9 +2100,8 @@ fn submesh_records(desc: &[u8]) -> Vec<(usize, usize)> {
let t = be32(desc, rel + 12);
if (3..=65535).contains(&a)
&& z == 0
&& c >= 3
&& c <= 200_000
&& c % 3 == 0
&& (3..=200_000).contains(&c)
&& c.is_multiple_of(3)
&& (1..=64).contains(&t)
{
out.push((a as usize, c as usize));
@@ -2151,7 +2164,7 @@ fn topology_report(
let mut agree = 0usize;
let mut counted = 0usize;
let mut edges: HashMap<(u32, u32), u32> = HashMap::new();
for t in indices.chunks_exact(3) {
for t in indices.as_chunks::<3>().0 {
let (a, b, c) = (t[0] as usize, t[1] as usize, t[2] as usize);
if a == b || b == c || a == c {
degen += 1;
@@ -2330,7 +2343,7 @@ pub fn submesh_albedos(bytes: &[u8], resource_name: &str) -> Vec<(usize, usize,
if (3..=65535).contains(&vtx)
&& z == 0
&& idx >= 3
&& idx % 3 == 0
&& idx.is_multiple_of(3)
&& idx < 400_000
&& tail > 0
&& tail < 0x10_0000
@@ -2423,7 +2436,7 @@ pub fn material_groups(bytes: &[u8], resource_name: &str, target: usize) -> Vec<
if (1..=70000).contains(&vtx)
&& tail == 4
&& cnt >= 3
&& cnt % 3 == 0
&& cnt.is_multiple_of(3)
&& cnt < 400_000
&& (off as usize) < 4_000_000
{

2
crates/sylpheed-formats/src/movie_voice.rs
View File

@@ -96,7 +96,7 @@ pub fn find_descriptor_before(buf: &[u8], before: usize) -> Option<usize> {
let mut o = (before - 1).min(cap) & !3;
loop {
let id = be(o);
if id >= 1 && id < ID_MAX && be(o + 4) == DESC_MARK && be(o + DESC_REPEAT) == id {
if (1..ID_MAX).contains(&id) && be(o + 4) == DESC_MARK && be(o + DESC_REPEAT) == id {
return Some(o);
}
if o < 4 {

4
crates/sylpheed-formats/src/ship_capture.rs
View File

@@ -287,7 +287,7 @@ pub fn parse_drawlog(text: &str) -> Vec<CapturedDraw> {
let mut pos: Vec<[f32; 3]> = Vec::new();
let mut consts: Vec<(usize, [f64; 4])> = Vec::new();
let mut flush = |base: u32,
let flush = |base: u32,
size: u32,
stride: u32,
pos: &mut Vec<[f32; 3]>,
@@ -426,7 +426,7 @@ pub fn correlate(
let Some((score, mirrored)) = pos_validate(d, &key.ref_pos) else {
continue; // positions disagree — not this part
};
if best.map_or(true, |(_, s, _)| score > s) {
if best.is_none_or(|(_, s, _)| score > s) {
best = Some((d, score, mirrored));
}
}

2
crates/sylpheed-formats/src/slb.rs
View File

@@ -517,7 +517,7 @@ pub fn to_xma_riff_best(slb: &[u8]) -> Option<Vec<u8>> {
while let Some(di) = find(slb, b"data", i) {
let declared = le32(slb, di + 4).unwrap_or(0) as usize;
let usable = declared.min(slb.len().saturating_sub(di + 8));
if best.map_or(true, |(_, b)| usable > b) {
if best.is_none_or(|(_, b)| usable > b) {
best = Some((di, usable));
}
i = di + 4;

22
crates/sylpheed-formats/src/texture.rs
View File

@@ -587,19 +587,19 @@ pub fn apply_endian_swap(data: &mut [u8], endianness: u8) {
match endianness {
1 => {
// k8in16 — swap the two bytes of each 16-bit half.
for c in data.chunks_exact_mut(2) {
for c in data.as_chunks_mut::<2>().0 {
c.swap(0, 1);
}
}
2 => {
// k8in32 — reverse each 32-bit word.
for c in data.chunks_exact_mut(4) {
for c in data.as_chunks_mut::<4>().0 {
c.reverse();
}
}
3 => {
// k16in32 — swap the two 16-bit halves of each 32-bit word.
for c in data.chunks_exact_mut(4) {
for c in data.as_chunks_mut::<4>().0 {
c.swap(0, 2);
c.swap(1, 3);
}
@@ -626,8 +626,8 @@ fn decode_surface(
} else {
// Linear layout — copy only the mip-0 slice.
let block_size = format.block_size() as u32;
let bw = ((width + block_size - 1) / block_size).max(1);
let bh = ((height + block_size - 1) / block_size).max(1);
let bw = width.div_ceil(block_size).max(1);
let bh = height.div_ceil(block_size).max(1);
let needed = bw as usize * bh as usize * format.bytes_per_block();
if raw_data.len() < needed {
return Err(TextureError::BufferTooSmall { needed, have: raw_data.len() });
@@ -651,7 +651,7 @@ fn decode_surface(
match format {
X360TextureFormat::Dxt1 => swap_bc_block_dwords(&mut linear_data),
X360TextureFormat::Dxt3 | X360TextureFormat::Dxt5 => {
for block in linear_data.chunks_exact_mut(16) {
for block in linear_data.as_chunks_mut::<16>().0 {
swap_bc_block_dwords(&mut block[8..16]);
}
}
@@ -666,8 +666,8 @@ fn decode_surface(
/// rounded up to the 4 KiB subresource alignment (`kTextureSubresourceAlignmentBytes`).
fn tiled_face_stride(width: u32, height: u32, format: X360TextureFormat) -> usize {
let bs = format.block_size() as u32;
let bw = ((width + bs - 1) / bs).max(1);
let bh = ((height + bs - 1) / bs).max(1);
let bw = width.div_ceil(bs).max(1);
let bh = height.div_ceil(bs).max(1);
let pitch_aligned = align_up(bw, STORAGE_ALIGN_BLOCKS).max(MACRO_TILE_BLOCKS);
let height_aligned = align_up(bh, STORAGE_ALIGN_BLOCKS).max(MACRO_TILE_BLOCKS);
let surface = pitch_aligned as usize * height_aligned as usize * format.bytes_per_block();
@@ -682,7 +682,7 @@ fn tiled_face_stride(width: u32, height: u32, format: X360TextureFormat) -> usiz
/// after the byte-level endian swap. Any trailing bytes that don't fill a full
/// 8-byte group are left untouched.
pub fn swap_bc_block_dwords(data: &mut [u8]) {
for unit in data.chunks_exact_mut(8) {
for unit in data.as_chunks_mut::<8>().0 {
// [d0 d1 d2 d3 | d4 d5 d6 d7] → [d4 d5 d6 d7 | d0 d1 d2 d3]
let (lo, hi) = unit.split_at_mut(4);
lo.swap_with_slice(hi);
@@ -742,8 +742,8 @@ pub fn detile(
let bpb_log2 = (bpb as u32).trailing_zeros();
// Visible dimensions in blocks, and the padded storage pitch/height.
let blocks_wide = ((width + block_size - 1) / block_size).max(1);
let blocks_tall = ((height + block_size - 1) / block_size).max(1);
let blocks_wide = width.div_ceil(block_size).max(1);
let blocks_tall = height.div_ceil(block_size).max(1);
let pitch_aligned = align_up(blocks_wide, STORAGE_ALIGN_BLOCKS).max(MACRO_TILE_BLOCKS);
let height_aligned = align_up(blocks_tall, STORAGE_ALIGN_BLOCKS).max(MACRO_TILE_BLOCKS);

2
crates/sylpheed-formats/src/ui_layout.rs
View File

@@ -910,7 +910,7 @@ pub fn compose(
// A dim backdrop stands in for the PRMD dim-quad + the live 3D scene behind
// an in-mission screen.
let mut canvas = vec![0u8; (w as usize) * (h as usize) * 4];
for px in canvas.chunks_exact_mut(4) {
for px in canvas.as_chunks_mut::<4>().0 {
px.copy_from_slice(&opts.backdrop);
}
let mut drawn = Vec::new();

2
crates/sylpheed-formats/src/vfs.rs
View File

@@ -237,7 +237,7 @@ pub fn decode_text(bytes: &[u8]) -> (String, &'static str) {
fn decode_utf16(bytes: &[u8], big_endian: bool) -> String {
let units: Vec<u16> = bytes
.chunks_exact(2)
.as_chunks::<2>().0.iter()
.map(|c| {
if big_endian {
u16::from_be_bytes([c[0], c[1]])

2
crates/sylpheed-formats/src/xiso.rs
View File

@@ -53,7 +53,7 @@ impl<F: Read + Seek + Send + Sync + 'static> XisoReader<F> {
info!(
"Opened XISO: root directory table at sector {}",
{ let s = volume.root_table.region.sector; s }
{ volume.root_table.region.sector }
);
Ok(Self { volume, file: wrapper })
}