From 6582e14ba5b4cd964101efad9481430ed87269aa Mon Sep 17 00:00:00 2001
From: Lachlan Deakin <ljdgit@gmail.com>
Date: Thu, 23 Jan 2025 09:15:55 +1100
Subject: [PATCH] fix: enforce `RawBytesOffsets` to be non-empty and add
 `.last()`

---
 zarrs/src/array/array_bytes.rs                | 24 +++++++---------
 .../array/array_bytes/raw_bytes_offsets.rs    | 28 +++++++++++++++++--
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/zarrs/src/array/array_bytes.rs b/zarrs/src/array/array_bytes.rs
index 34f37995..42210075 100644
--- a/zarrs/src/array/array_bytes.rs
+++ b/zarrs/src/array/array_bytes.rs
@@ -76,18 +76,13 @@ impl<'a> ArrayBytes<'a> {
         offsets: RawBytesOffsets<'a>,
     ) -> Result<Self, RawBytesOffsetsOutOfBoundsError> {
         let bytes = bytes.into();
-        match offsets.last() {
-            Some(&last) => {
-                if last <= bytes.len() {
-                    Ok(Self::Variable(bytes, offsets))
-                } else {
-                    Err(RawBytesOffsetsOutOfBoundsError {
-                        offset: last,
-                        len: bytes.len(),
-                    })
-                }
-            }
-            None => Err(RawBytesOffsetsOutOfBoundsError { offset: 0, len: 0 }),
+        if offsets.last() <= bytes.len() {
+            Ok(Self::Variable(bytes, offsets))
+        } else {
+            Err(RawBytesOffsetsOutOfBoundsError {
+                offset: offsets.last(),
+                len: bytes.len(),
+            })
         }
     }
 
@@ -100,7 +95,7 @@ impl<'a> ArrayBytes<'a> {
         offsets: RawBytesOffsets<'a>,
     ) -> Self {
         let bytes = bytes.into();
-        debug_assert!(offsets.last().is_some_and(|&last| last <= bytes.len()));
+        debug_assert!(offsets.last() <= bytes.len());
         Self::Variable(bytes, offsets)
     }
 
@@ -501,7 +496,7 @@ pub(crate) fn merge_chunks_vlen<'a>(
 
     // Write bytes
     // TODO: Go parallel
-    let mut bytes = vec![0; *offsets.last().unwrap()];
+    let mut bytes = vec![0; offsets.last()];
     for (chunk_bytes, chunk_subset) in chunk_bytes_and_subsets {
         let (chunk_bytes, chunk_offsets) = chunk_bytes.into_variable()?;
         let indices = chunk_subset.linearised_indices(array_shape).unwrap();
@@ -664,6 +659,7 @@ mod tests {
     #[test]
     fn array_bytes_vlen() {
         let data = [0u8, 1, 2, 3, 4];
+        assert!(ArrayBytes::new_vlen(&data, vec![0].try_into().unwrap()).is_ok());
         assert!(ArrayBytes::new_vlen(&data, vec![0, 5].try_into().unwrap()).is_ok());
         assert!(ArrayBytes::new_vlen(&data, vec![0, 5, 5].try_into().unwrap()).is_ok());
         assert!(ArrayBytes::new_vlen(&data, vec![0, 5, 6].try_into().unwrap()).is_err());
diff --git a/zarrs/src/array/array_bytes/raw_bytes_offsets.rs b/zarrs/src/array/array_bytes/raw_bytes_offsets.rs
index 4b335014..5adbb795 100644
--- a/zarrs/src/array/array_bytes/raw_bytes_offsets.rs
+++ b/zarrs/src/array/array_bytes/raw_bytes_offsets.rs
@@ -21,7 +21,14 @@ impl Deref for RawBytesOffsets<'_> {
 ///
 /// This error occurs when the offsets are not monotonically increasing.
 #[derive(Debug, Error, Display)]
-pub struct RawBytesOffsetsCreateError;
+pub enum RawBytesOffsetsCreateError {
+    /// The length must be greater than zero.
+    #[display("length must be greater than zero")]
+    ZeroLength,
+    /// The offsets are not monotonically increasing.
+    #[display("offsets are not monotonically increasing")]
+    NotMonotonicallyIncreasing,
+}
 
 impl<'a> RawBytesOffsets<'a> {
     /// Creates a new `RawBytesOffsets`.
@@ -30,10 +37,12 @@ impl<'a> RawBytesOffsets<'a> {
     /// Returns an error if the offsets are not monotonically increasing.
     pub fn new(offsets: impl Into<Cow<'a, [usize]>>) -> Result<Self, RawBytesOffsetsCreateError> {
         let offsets = offsets.into();
-        if offsets.windows(2).all(|w| w[1] >= w[0]) {
+        if offsets.is_empty() {
+            Err(RawBytesOffsetsCreateError::ZeroLength)
+        } else if offsets.windows(2).all(|w| w[1] >= w[0]) {
             Ok(Self(offsets))
         } else {
-            Err(RawBytesOffsetsCreateError)
+            Err(RawBytesOffsetsCreateError::NotMonotonicallyIncreasing)
         }
     }
 
@@ -44,6 +53,7 @@ impl<'a> RawBytesOffsets<'a> {
     #[must_use]
     pub unsafe fn new_unchecked(offsets: impl Into<Cow<'a, [usize]>>) -> Self {
         let offsets = offsets.into();
+        debug_assert!(!offsets.is_empty());
         debug_assert!(offsets.windows(2).all(|w| w[1] >= w[0]));
         Self(offsets)
     }
@@ -53,6 +63,15 @@ impl<'a> RawBytesOffsets<'a> {
     pub fn into_owned(self) -> RawBytesOffsets<'static> {
         RawBytesOffsets(self.0.into_owned().into())
     }
+
+    /// Returns the last offset.
+    #[must_use]
+    pub fn last(&self) -> usize {
+        unsafe {
+            // SAFETY: The offsets cannot be empty.
+            *self.0.last().unwrap_unchecked()
+        }
+    }
 }
 
 impl<'a> TryFrom<Cow<'a, [usize]>> for RawBytesOffsets<'a> {
@@ -95,6 +114,9 @@ mod tests {
     fn raw_bytes_offsets() {
         let offsets = RawBytesOffsets::new(vec![0, 1, 2, 3]).unwrap();
         assert_eq!(&*offsets, &[0, 1, 2, 3]);
+        assert!(RawBytesOffsets::new(vec![]).is_err());
+        assert!(RawBytesOffsets::new(vec![0]).is_ok());
+        assert!(RawBytesOffsets::new(vec![10]).is_ok()); // nonsense, but not invalid
         assert!(RawBytesOffsets::new(vec![0, 1, 1]).is_ok());
         assert!(RawBytesOffsets::new(vec![0, 1, 0]).is_err());
         assert!(RawBytesOffsets::try_from(vec![0, 1, 2]).is_ok());