fix unsoundness

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "fimg"
-version = "0.2.2"
+version = "0.3.0"
 authors = ["bend-n <bend.n@outlook.com>"]
 license = "MIT"
 edition = "2021"

benches/affine_transformations.rs

@@ -7,11 +7,8 @@ macro_rules! bench {
     (fn $name: ident() { run $fn: ident() }) => {
         #[bench]
         fn $name(b: &mut Bencher) {
-            let mut img: Image<_, 4> = Image::new(
+            let mut img: Image<_, 4> =
-                64.try_into().unwrap(),
+                Image::build(64, 64).buf(include_bytes!("4_180x180.imgbuf").to_vec());
-                64.try_into().unwrap(),
-                include_bytes!("4_180x180.imgbuf").to_vec(),
-            );
             #[allow(unused_unsafe)]
             b.iter(|| unsafe {
                 for _ in 0..256 {

benches/overlays.rs

@@ -6,16 +6,8 @@ use test::Bencher;
 #[bench]
 fn overlay_3on3at(bench: &mut Bencher) {
     let mut v = vec![0u8; 3 * 64 * 64];
-    let mut a: Image<_, 3> = Image::new(
+    let mut a: Image<_, 3> = Image::build(64, 64).buf(v.as_mut_slice());
-        64.try_into().unwrap(),
+    let b: Image<&[u8], 3> = Image::build(4, 4).buf(include_bytes!("3_4x4.imgbuf"));
-        64.try_into().unwrap(),
-        v.as_mut_slice(),
-    );
-    let b = Image::<&[u8], 3>::new(
-        4.try_into().unwrap(),
-        4.try_into().unwrap(),
-        *&include_bytes!("3_4x4.imgbuf"),
-    );
     bench.iter(|| unsafe {
         for x in 0..16 {
             for y in 0..16 {
@@ -23,17 +15,13 @@ fn overlay_3on3at(bench: &mut Bencher) {
             }
         }
     });
-    assert_eq!(a.as_ref().buffer, include_bytes!("3x3_at_out.imgbuf"));
+    assert_eq!(a.as_ref().buffer(), include_bytes!("3x3_at_out.imgbuf"));
 }
 
 #[bench]
 fn overlay_4on3at(bench: &mut Bencher) {
     let mut a: Image<_, 3> = Image::alloc(64, 64);
-    let b = Image::<&[u8], 4>::new(
+    let b: Image<&[u8], 4> = Image::build(4, 4).buf(include_bytes!("4_4x4.imgbuf"));
-        4.try_into().unwrap(),
-        4.try_into().unwrap(),
-        *&include_bytes!("4_4x4.imgbuf"),
-    );
     bench.iter(|| unsafe {
         for x in 0..16 {
             for y in 0..16 {
@@ -41,17 +29,13 @@ fn overlay_4on3at(bench: &mut Bencher) {
             }
         }
     });
-    assert_eq!(a.as_ref().buffer, include_bytes!("4x3_at_out.imgbuf"));
+    assert_eq!(a.as_ref().buffer(), include_bytes!("4x3_at_out.imgbuf"));
 }
 
 #[bench]
 fn overlay_4on4at(bench: &mut Bencher) {
     let mut a: Image<_, 4> = Image::alloc(64, 64);
-    let b = Image::<&[u8], 4>::new(
+    let b: Image<&[u8], 4> = Image::build(4, 4).buf(include_bytes!("4_4x4.imgbuf"));
-        4.try_into().unwrap(),
-        4.try_into().unwrap(),
-        *&include_bytes!("4_4x4.imgbuf"),
-    );
     bench.iter(|| unsafe {
         for x in 0..16 {
             for y in 0..16 {
@@ -59,5 +43,5 @@ fn overlay_4on4at(bench: &mut Bencher) {
             }
         }
     });
-    assert_eq!(a.as_ref().buffer, include_bytes!("4x4_at_out.imgbuf"));
+    assert_eq!(a.as_ref().buffer(), include_bytes!("4x4_at_out.imgbuf"));
 }

src/builder.rs

@@ -0,0 +1,94 @@
+//! safe builder for the image
+//!
+//! does not let you do funny things
+use std::marker::PhantomData;
+
+use crate::Image;
+
+impl<B: buf::Buffer, const C: usize> Image<B, C> {
+    /// creates a builder
+    pub const fn build(w: u32, h: u32) -> Builder<B, C> {
+        Builder::new(w, h)
+    }
+}
+
+/// Safe [Image] builder.
+pub struct Builder<B, const C: usize> {
+    /// the width in a zeroable type. zeroable so as to make the check in [`buf`] easier.
+    width: u32,
+    /// the height in a zeroable type.
+    height: u32,
+    #[allow(clippy::missing_docs_in_private_items)]
+    _buffer: PhantomData<B>,
+}
+impl<B: buf::Buffer, const C: usize> Builder<B, C> {
+    /// create new builder
+    pub const fn new(w: u32, h: u32) -> Self {
+        Self {
+            width: w,
+            height: h,
+            _buffer: PhantomData,
+        }
+    }
+
+    /// apply a buffer, and build
+    pub fn buf(self, buffer: B) -> Image<B, C> {
+        if buffer.len() as u32 != C as u32 * self.width * self.height {
+            panic!("invalid buffer size");
+        }
+        Image {
+            buffer,
+            width: self.width.try_into().expect("passed zero width to builder"),
+            height: self
+                .height
+                .try_into()
+                .expect("passed zero height to builder"),
+        }
+    }
+}
+
+impl<const C: usize> Builder<Vec<u8>, C> {
+    /// allocate this image
+    pub fn alloc(self) -> Image<Vec<u8>, C> {
+        Image::alloc(self.width, self.height)
+    }
+}
+
+/// seals the [`Buffer`] trait
+mod buf {
+    /// A valid buffer for use in the builder
+    pub trait Buffer {
+        #[doc(hidden)]
+        fn len(&self) -> usize;
+    }
+    impl<T> Buffer for Vec<T> {
+        fn len(&self) -> usize {
+            self.len()
+        }
+    }
+    impl<T> Buffer for &[T] {
+        fn len(&self) -> usize {
+            <[T]>::len(self)
+        }
+    }
+    impl<T> Buffer for &mut [T] {
+        fn len(&self) -> usize {
+            <[T]>::len(self)
+        }
+    }
+    impl<T, const N: usize> Buffer for [T; N] {
+        fn len(&self) -> usize {
+            N
+        }
+    }
+    impl<T, const N: usize> Buffer for &[T; N] {
+        fn len(&self) -> usize {
+            N
+        }
+    }
+    impl<T, const N: usize> Buffer for &mut [T; N] {
+        fn len(&self) -> usize {
+            N
+        }
+    }
+}

src/lib.rs

@@ -24,6 +24,7 @@
 use std::{num::NonZeroU32, slice::SliceIndex};
 
 mod affine;
+pub mod builder;
 mod overlay;
 pub use overlay::{Overlay, OverlayAt};
 
@@ -77,11 +78,11 @@ unsafe fn really_unsafe_index(x: u32, y: u32, w: u32) -> usize {
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Image<T, const CHANNELS: usize> {
     /// column order 2d slice/vec
-    pub buffer: T,
+    buffer: T,
     /// image horizontal size
-    pub width: NonZeroU32,
+    width: NonZeroU32,
     /// image vertical size
-    pub height: NonZeroU32,
+    height: NonZeroU32,
 }
 
 impl<const CHANNELS: usize> Default for Image<&'static [u8], CHANNELS> {
@@ -109,13 +110,33 @@ impl<T, const CHANNELS: usize> Image<T, CHANNELS> {
 
     #[inline]
     /// create a new image
-    pub const fn new(width: NonZeroU32, height: NonZeroU32, buffer: T) -> Self {
+    ///
+    /// # Safety
+    ///
+    /// does not check that buffer.len() == w * h * C
+    ///
+    /// using this with invalid values may result in future UB
+    pub const unsafe fn new(width: NonZeroU32, height: NonZeroU32, buffer: T) -> Self {
         Self {
             buffer,
             width,
             height,
         }
     }
+
+    /// returns a immutable reference to the backing buffer
+    pub const fn buffer(&self) -> &T {
+        &self.buffer
+    }
+
+    /// returns a mutable(!) reference to the backing buffer
+    ///
+    /// # Safety
+    ///
+    /// please do not change buffer size.
+    pub unsafe fn buffer_mut(&mut self) -> &mut T {
+        &mut self.buffer
+    }
 }
 
 impl<const CHANNELS: usize> Image<&[u8], CHANNELS> {
@@ -216,28 +237,38 @@ impl<T: std::ops::DerefMut<Target = [u8]>, const CHANNELS: usize> Image<T, CHANN
 impl<const CHANNELS: usize> Image<&mut [u8], CHANNELS> {
     /// Downcast the mutable reference
     pub fn as_ref(&self) -> Image<&[u8], CHANNELS> {
-        Image::new(self.width, self.height, self.buffer)
+        // SAFETY: we got constructed okay, parameters must be valid
+        unsafe { Image::new(self.width, self.height, self.buffer) }
     }
 }
 
 impl<const CHANNELS: usize> Image<&mut [u8], CHANNELS> {
     /// Copy this ref image
     pub fn copy(&mut self) -> Image<&mut [u8], CHANNELS> {
-        Image::new(self.width, self.height, self.buffer)
+        #[allow(clippy::undocumented_unsafe_blocks)]
+        unsafe {
+            Image::new(self.width, self.height, self.buffer)
+        }
     }
 }
 
 impl<const CHANNELS: usize> Image<Vec<u8>, CHANNELS> {
     /// Create a reference to this owned image
     pub fn as_ref(&self) -> Image<&[u8], CHANNELS> {
-        Image::new(self.width, self.height, &self.buffer)
+        #[allow(clippy::undocumented_unsafe_blocks)]
+        unsafe {
+            Image::new(self.width, self.height, &self.buffer)
+        }
     }
 }
 
 impl<const CHANNELS: usize> Image<Vec<u8>, CHANNELS> {
     /// Create a mutable reference to this owned image
     pub fn as_mut(&mut self) -> Image<&mut [u8], CHANNELS> {
-        Image::new(self.width, self.height, &mut self.buffer)
+        #[allow(clippy::undocumented_unsafe_blocks)]
+        unsafe {
+            Image::new(self.width, self.height, &mut self.buffer)
+        }
     }
 }
 
@@ -302,14 +333,9 @@ save!(1 == Grayscale("Y"));
 
 #[cfg(test)]
 macro_rules! img {
-    [[$($v:literal),+] [$($v2:literal),+]] => {{
+    [[$($v:literal),+] [$($v2:literal),+]] => {
-        let from: Image<Vec<u8>, 1> = Image::new(
+        Image::<Vec<u8>, 1>::build(2,2).buf(vec![$($v,)+ $($v2,)+])
-            2.try_into().unwrap(),
+    }
-            2.try_into().unwrap(),
-            vec![$($v,)+ $($v2,)+]
-        );
-        from
-    }}
 }
 #[cfg(test)]
 use img;

src/overlay.rs

@@ -131,6 +131,8 @@ impl OverlayAt<Image<&[u8], 3>> for Image<&mut [u8], 3> {
                     let o_x = ((j + y as usize) * self.width() as usize + x as usize) * 3
                         ..((j + y as usize) * self.width() as usize + x as usize + ($n as usize))
                             * 3;
+                    debug_assert!(o_x.end < self.buffer().len());
+                    debug_assert!(i_x.end < with.buffer().len());
                     // SAFETY: bounds are ✅
                     let a = unsafe { self.buffer.get_unchecked_mut(o_x) };
                     // SAFETY: we are in ⬜!