From: Ian Jackson Date: Sun, 21 Feb 2021 19:15:48 +0000 (+0000) Subject: Use checked arithmetic on coordinates X-Git-Tag: otter-0.4.0~331 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=1536529ba128710499b2952b5855b3e5c3642932;p=otter.git Use checked arithmetic on coordinates In particular, prevent use of unchecked arithmetic on PosC. Signed-off-by: Ian Jackson --- diff --git a/daemon/cmdlistener.rs b/daemon/cmdlistener.rs index daf3ce55..4e4e3cb7 100644 --- a/daemon/cmdlistener.rs +++ b/daemon/cmdlistener.rs @@ -623,7 +623,7 @@ fn execute_game_insn<'cs, 'igr, 'ig: 'igr>( let piece = gs.pieces.as_mut(modperm).insert(pc); ig.ipieces.as_mut(modperm).insert(piece, p); updates.push((piece, PieceUpdateOp::Insert(()))); - pos += posd; + pos = (pos + posd)?; } (U{ pcs: updates, diff --git a/src/bin/otter.rs b/src/bin/otter.rs index 3027f3b8..7fe0868a 100644 --- a/src/bin/otter.rs +++ b/src/bin/otter.rs @@ -1245,7 +1245,7 @@ mod library_add { fn place(&mut self, bbox: &[Pos;2], pieces: &Vec, ma: &MainOpts) -> Option { - let PosC([w,h]) = bbox[1] - bbox[0]; + let PosC([w,h]) = (bbox[1] - bbox[0])?; let mut did_newline = false; let (ncbot, tlhs) = 'search: loop { @@ -1261,8 +1261,8 @@ mod library_add { if let Some((nclhs, clash_bot)) = pieces.iter() .filter_map(|p| (|| if_chain! { if let Some(pv) = p.visible.as_ref(); - let tl = pv.pos + pv.bbox[0]; - let br = pv.pos + pv.bbox[1]; + let tl = (pv.pos + pv.bbox[0])?; + let br = (pv.pos + pv.bbox[1])?; if !(tl.0[0] >= self.clhs || tl.0[1] >= ncbot || br.0[0] <= tlhs @@ -1304,7 +1304,7 @@ mod library_add { }; self.cbot = ncbot; let ttopleft = PosC([tlhs, self.top]); - let tnominal = ttopleft - bbox[0]; + let tnominal = (ttopleft - bbox[0])?; if ma.verbose > 3 { dbg!(&self, &tnominal); } Some(tnominal) diff --git a/src/commands.rs b/src/commands.rs index 0122afb8..72fd60e0 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -185,6 +185,7 @@ pub enum MgmtError { BadGlob { pat: String, msg: String }, BadSpec(#[from] SpecError), TokenDeliveryFailed(#[from] TokenDeliveryError), + CoordinateOverflow(#[from] CoordinateOverflow), } impl Display for MgmtError { #[throws(fmt::Error)] diff --git a/src/error.rs b/src/error.rs index e6a33f38..dd12b80e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -53,6 +53,8 @@ pub enum InternalError { Anyhow(#[from] anyhow::Error), #[error("Game contains only partial data for player, or account missing")] PartialPlayerData, + #[error("Coordinate overflow")] + CoordinateOverflow(#[from] CoordinateOverflow), #[error("Multiple errors occurred where only one could be reported")] Aggregated, } diff --git a/src/shapelib.rs b/src/shapelib.rs index c27b65cf..6f193e69 100644 --- a/src/shapelib.rs +++ b/src/shapelib.rs @@ -541,7 +541,7 @@ pub struct Rectangle { pub xy: PosC } impl Outline for Rectangle { #[throws(IE)] fn outline_path(&self, _pri: &PieceRenderInstructions, scale: f64) -> Html { - let xy = self.xy * scale; + let xy = (self.xy * scale)?; svg_rectangle_path(xy)? } #[throws(IE)] @@ -556,7 +556,7 @@ impl Outline for Rectangle { let pos: Pos = self.xy.map( |v| ((v * 0.5).ceil()) as Coord ); - let neg = -pos; + let neg = (-pos)?; [ neg, pos ] } } diff --git a/src/spec.rs b/src/spec.rs index e1f7f9a6..a2f8e499 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -248,63 +248,62 @@ pub mod piece_specs { //---------- Pos ---------- pub mod pos_traits { - use std::ops::{Add,Sub,Mul,Neg,AddAssign,SubAssign}; + use std::ops::{Add,Sub,Mul,Neg}; use crate::prelude::*; - impl+Copy+Clone+Debug> Add> for PosC { - type Output = PosC; + impl Add> for PosC { + type Output = Result; + #[throws(CoordinateOverflow)] fn add(self, rhs: PosC) -> PosC { PosC( itertools::zip_eq( self.0.iter().cloned(), rhs .0.iter().cloned(), - ).map(|(a,b)| a + b) - .collect::>().into_inner().unwrap() + ).map( + |(a,b)| a.checked_add(b) + ) + .collect::,_>>()? + .into_inner().unwrap() ) } } - impl+Copy+Clone+Debug> Sub> for PosC { - type Output = PosC; + impl Sub> for PosC { + type Output = Result; + #[throws(CoordinateOverflow)] fn sub(self, rhs: PosC) -> PosC { PosC( itertools::zip_eq( self.0.iter().cloned(), rhs .0.iter().cloned(), - ).map(|(a,b)| a - b) - .collect::>().into_inner().unwrap() + ).map(|(a,b)| a.checked_sub(b)) + .collect::,_>>()? + .into_inner().unwrap() ) } } - impl+Copy+Clone+Debug> AddAssign> for PosC { - fn add_assign(&mut self, rhs: PosC) { - *self = *self + rhs; - } - } - - impl+Copy+Clone+Debug> SubAssign> for PosC { - fn sub_assign(&mut self, rhs: PosC) { - *self = *self - rhs; - } - } - - impl+Copy+Clone+Debug> Mul for PosC { - type Output = PosC; - fn mul(self, rhs: T) -> PosC { + impl> Mul for PosC { + type Output = Result; + #[throws(CoordinateOverflow)] + fn mul(self, rhs: S) -> PosC { PosC( - self.0.iter().cloned().map(|a| a * rhs) - .collect::>().into_inner().unwrap() + self.0.iter().cloned().map( + |a| a.checked_mul(rhs) + ) + .collect::,_>>()? + .into_inner().unwrap() ) } } - impl+Copy+Clone+Debug> Neg for PosC { - type Output = Self; + impl Neg for PosC { + type Output = Result; + #[throws(CoordinateOverflow)] fn neg(self) -> Self { PosC( - self.0.iter().cloned().map(|a| -a) - .collect::>().into_inner().unwrap() + self.0.iter().cloned().map(|a| a.checked_neg()) + .collect::,_>>()?.into_inner().unwrap() ) } } diff --git a/wdriver/wdt-simple.rs b/wdriver/wdt-simple.rs index a6e76731..83496514 100644 --- a/wdriver/wdt-simple.rs +++ b/wdriver/wdt-simple.rs @@ -182,7 +182,7 @@ impl Ctx { let mut w = su.w(window)?; let p = w.find_piece(pc)?; let start = p.posg()?; - let try_end = start + PosC([dx, 0]); + let try_end = (start + PosC([dx, 0]))?; let (sx,sy) = w.posg2posw(start)?; w.action_chain()