Use checked arithmetic on coordinates

In particular, prevent use of unchecked arithmetic on PosC.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/daemon/cmdlistener.rs b/daemon/cmdlistener.rs
index daf3ce5549ec438a23755cb176c8590520977d99..4e4e3cb7dd8d21a6adb4c6ee5499dea24fab465e 100644 (file)
--- 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 3027f3b84b5e677be1f969b2b3dc06c638eaddf9..7fe0868a303b1e4b60965121a3a7bfeab9f8cf0d 100644 (file)
--- 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<MgmtGamePieceInfo>, ma: &MainOpts)
                -> Option<Pos> {
-        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 0122afb8a3d8e41a067e80b175e7430dd1f1ce56..72fd60e0008dda3f20425cf621d4bd887df52863 100644 (file)
--- 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 e6a33f381b1c2660adfb082b769ee3fc6b7e0495..dd12b80e889c2b00195d991faff060f70abab3d3 100644 (file)
--- 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 c27b65cfe3b63176cafdcbea26adf6685a2391a8..6f193e697f07d717f4f88b4928a83f56733bf584 100644 (file)
--- a/src/shapelib.rs
+++ b/src/shapelib.rs
@@ -541,7 +541,7 @@ pub struct Rectangle { pub xy: PosC<f64> }
 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 e1f7f9a6a76352070cd8104333c389cf25d0ff20..a2f8e4992449cb9e73644db406dedc3310e240fb 100644 (file)
--- 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<T:Add<T,Output=T>+Copy+Clone+Debug> Add<PosC<T>> for PosC<T> {
-    type Output = PosC<T>;
+  impl<T:CheckedArith> Add<PosC<T>> for PosC<T> {
+    type Output = Result<Self, CoordinateOverflow>;
+    #[throws(CoordinateOverflow)]
     fn add(self, rhs: PosC<T>) -> PosC<T> {
       PosC(
         itertools::zip_eq(
           self.0.iter().cloned(),
           rhs .0.iter().cloned(),
-        ).map(|(a,b)| a + b)
-          .collect::<ArrayVec<_>>().into_inner().unwrap()
+        ).map(
+          |(a,b)| a.checked_add(b)
+        )
+          .collect::<Result<ArrayVec<_>,_>>()?
+          .into_inner().unwrap()
       )
     }
   }
 
-  impl<T:Sub<T,Output=T>+Copy+Clone+Debug> Sub<PosC<T>> for PosC<T> {
-    type Output = PosC<T>;
+  impl<T:CheckedArith> Sub<PosC<T>> for PosC<T> {
+    type Output = Result<Self, CoordinateOverflow>;
+    #[throws(CoordinateOverflow)]
     fn sub(self, rhs: PosC<T>) -> PosC<T> {
       PosC(
         itertools::zip_eq(
           self.0.iter().cloned(),
           rhs .0.iter().cloned(),
-        ).map(|(a,b)| a - b)
-          .collect::<ArrayVec<_>>().into_inner().unwrap()
+        ).map(|(a,b)| a.checked_sub(b))
+          .collect::<Result<ArrayVec<_>,_>>()?
+          .into_inner().unwrap()
       )
     }
   }
 
-  impl<T:Add<T,Output=T>+Copy+Clone+Debug> AddAssign<PosC<T>> for PosC<T> {
-    fn add_assign(&mut self, rhs: PosC<T>) {
-      *self = *self + rhs;
-    }
-  }
-
-  impl<T:Sub<T,Output=T>+Copy+Clone+Debug> SubAssign<PosC<T>> for PosC<T> {
-    fn sub_assign(&mut self, rhs: PosC<T>) {
-      *self = *self - rhs;
-    }
-  }
-
-  impl<T:Mul<T,Output=T>+Copy+Clone+Debug> Mul<T> for PosC<T> {
-    type Output = PosC<T>;
-    fn mul(self, rhs: T) -> PosC<T> {
+  impl<S:Copy+Debug+Clone+'static,T:CheckedArithMul<S>> Mul<S> for PosC<T> {
+    type Output = Result<Self, CoordinateOverflow>;
+    #[throws(CoordinateOverflow)]
+    fn mul(self, rhs: S) -> PosC<T> {
       PosC(
-        self.0.iter().cloned().map(|a| a * rhs)
-          .collect::<ArrayVec<_>>().into_inner().unwrap()
+        self.0.iter().cloned().map(
+          |a| a.checked_mul(rhs)
+        )
+          .collect::<Result<ArrayVec<_>,_>>()?
+          .into_inner().unwrap()
       )
     }
   }
 
-  impl<T:Neg<Output=T>+Copy+Clone+Debug> Neg for PosC<T> {
-    type Output = Self;
+  impl<T:CheckedArith> Neg for PosC<T> {
+    type Output = Result<Self, CoordinateOverflow>;
+    #[throws(CoordinateOverflow)]
     fn neg(self) -> Self {
       PosC(
-        self.0.iter().cloned().map(|a| -a)
-          .collect::<ArrayVec<_>>().into_inner().unwrap()
+        self.0.iter().cloned().map(|a| a.checked_neg())
+          .collect::<Result<ArrayVec<_>,_>>()?.into_inner().unwrap()
       )
     }
   }

diff --git a/wdriver/wdt-simple.rs b/wdriver/wdt-simple.rs
index a6e76731fa1291785a7e963501c9b87605f0247b..834965145a29b8ba0925ed3d8ee094bc653c4404 100644 (file)
--- 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()