From fc4157a1e07be0c26ea5ab3dabfc8fddb7892eb8 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 29 Aug 2026 22:07:05 +0200 Subject: [PATCH] Keep the test scene's own arithmetic from overflowing a u64 The first compile this branch ever had. `cargo fmt` reflowed four files and clippy passed at -D warnings untouched, but one test panicked: `the_signature_does_not_change_with_scale`, on "attempt to multiply with overflow". It is the fixture, not the feature. `scene()`'s little LCG multiplied the block's y by the golden-ratio constant with a plain `*` while the term beside it already used `wrapping_mul`, so any scene taller than about 104 pixels overflowed in debug. Only the scale test builds one that large, which is why 345 of 346 passed around it. Co-Authored-By: Claude Opus 5 (1M context) --- core/dr-catalog/src/bursts.rs | 27 ++++++++++++++++++--------- ui/dr-ui/src/bursts.rs | 6 +----- ui/dr-ui/src/library.rs | 6 +----- ui/dr-ui/src/library_ui.rs | 4 +++- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/core/dr-catalog/src/bursts.rs b/core/dr-catalog/src/bursts.rs index 3aa628e..7402579 100644 --- a/core/dr-catalog/src/bursts.rs +++ b/core/dr-catalog/src/bursts.rs @@ -609,7 +609,9 @@ pub fn members(conn: &Connection, burst: ImageId) -> Result, Catalo WHERE bm.burst_id = ?1 ORDER BY i.captured_at ASC, i.id ASC", )?; - let rows = stmt.query_map([burst.0 as i64], |r| Ok(ImageId(r.get::<_, i64>(0)? as u64)))?; + let rows = stmt.query_map([burst.0 as i64], |r| { + Ok(ImageId(r.get::<_, i64>(0)? as u64)) + })?; Ok(rows.collect::, _>>()?) } @@ -754,7 +756,7 @@ mod tests { let mut s = seed .wrapping_add(bx as u64) .wrapping_mul(6_364_136_223_846_793_005) - .wrapping_add(by as u64 * 1_442_695_040_888_963_407); + .wrapping_add((by as u64).wrapping_mul(1_442_695_040_888_963_407)); s ^= s >> 33; let v = (s % 256) as i32 + brighter; out[y * w + x] = v.clamp(0, 255) as u8; @@ -836,7 +838,10 @@ mod tests { // The top bit is the one at risk: SQLite integers are signed. let s = Signature(u64::MAX); assert_eq!(Signature::from_stored(s.to_stored()), s); - assert_eq!(Signature::from_stored(Signature(0).to_stored()), Signature(0)); + assert_eq!( + Signature::from_stored(Signature(0).to_stored()), + Signature(0) + ); } // ── the grouping ────────────────────────────────────────────────────── @@ -844,7 +849,11 @@ mod tests { #[test] fn frames_a_second_apart_and_alike_are_one_burst() { let g = group( - &[frame(1, 1000, 0xFF00), frame(2, 1001, 0xFF00), frame(3, 1001, 0xFF01)], + &[ + frame(1, 1000, 0xFF00), + frame(2, 1001, 0xFF00), + frame(3, 1001, 0xFF01), + ], Rules::default(), ); assert_eq!(g, vec![vec![ImageId(1), ImageId(2), ImageId(3)]]); @@ -866,10 +875,7 @@ mod tests { ); assert_eq!( g, - vec![ - vec![ImageId(1), ImageId(2)], - vec![ImageId(3), ImageId(4)], - ], + vec![vec![ImageId(1), ImageId(2)], vec![ImageId(3), ImageId(4)],], "the three-second pause did not end the first burst" ); } @@ -1109,7 +1115,10 @@ mod tests { choose_representative(cat.connection(), ImageId(2)).unwrap(); let m = memberships(cat.connection(), &[ImageId(2)]).unwrap(); - assert!(m[&ImageId(2)].representative, "the pick did not take effect"); + assert!( + m[&ImageId(2)].representative, + "the pick did not take effect" + ); regroup(cat.connection(), Rules::default()).unwrap(); let m = memberships(cat.connection(), &[ImageId(1), ImageId(2)]).unwrap(); diff --git a/ui/dr-ui/src/bursts.rs b/ui/dr-ui/src/bursts.rs index 5ec1083..b83253b 100644 --- a/ui/dr-ui/src/bursts.rs +++ b/ui/dr-ui/src/bursts.rs @@ -179,11 +179,7 @@ thread_local! { /// Deliberately silent otherwise. The sweeps around it report progress because /// they run for tens of minutes; this is seconds, and a status line for it would /// be a line the user must read in order to learn nothing. -pub fn start_pass( - catalog_path: PathBuf, - thumbs_dir: PathBuf, - grouped: impl Fn(usize) + 'static, -) { +pub fn start_pass(catalog_path: PathBuf, thumbs_dir: PathBuf, grouped: impl Fn(usize) + 'static) { // A second pass would read the same rows and write the same answer over the // first one's transactions. if RUNNING.get() { diff --git a/ui/dr-ui/src/library.rs b/ui/dr-ui/src/library.rs index 943937f..97a81d9 100644 --- a/ui/dr-ui/src/library.rs +++ b/ui/dr-ui/src/library.rs @@ -4916,11 +4916,7 @@ mod tests { "UPDATE images SET captured_at = ?2, camera = 'Canon EOS R5', perceptual_hash = ?3 WHERE id = ?1", - rusqlite::params![ - id.0 as i64, - 1_000 + n as i64, - Signature(hash).to_stored() - ], + rusqlite::params![id.0 as i64, 1_000 + n as i64, Signature(hash).to_stored()], ) .unwrap(); } diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index a92f67c..7d5383a 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -3790,7 +3790,9 @@ fn start_thumbnail_sweep(window: &AppWindow, ctl: &Rc) { library::catalog_path(&conn.account), library::thumbs_dir(&conn.account), move |bursts| { - let Some(w) = weak_after.upgrade() else { return }; + let Some(w) = weak_after.upgrade() else { + return; + }; if bursts > 0 && w.get_show_library() { schedule_reload(&w, &ctl_after); }