Restore the whole cursor on failed read transactions; clarify docs

CHANGELOG.md

@@ -1,6 +1,8 @@
 ### 0.5.1 ###
 * :bug: `ReadCursor::transaction` closures can now return data borrowed from the input. See [#12](https://github.com/stepfunc/scursor/issues/12).
 * :bug: `WriteCursor::at_pos` now accepts `FnOnce` closures. See [#13](https://github.com/stepfunc/scursor/issues/13).
+* :bug: `ReadCursor::transaction` now restores the whole cursor on error, not just its position. See [#16](https://github.com/stepfunc/scursor/issues/16).
+* :book: Documented that write transactions restore only the position, not buffer contents. Corrected the `TrailingBytes::count` and `WriteCursor::written_since` docs.
 * :wrench: Replaced the Kani proofs with link-time `no_panic` checks, run in CI, covering every public routine for all inputs. A test fails if a new routine is left out of the check. A canary test confirms the check still detects panics.
 * :wrench: Added round-trip property tests and a CI workflow.
 * Declared the minimum supported Rust version (1.77).

src/read.rs

@@ -15,7 +15,7 @@ pub struct ReadError;
 /// Error when asserting that there are no remaining bytes
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub struct TrailingBytes {
-    /// number of non-zero trailing bytes
+    /// number of trailing bytes
     pub count: core::num::NonZeroUsize,
 }
 
@@ -64,18 +64,18 @@ impl<'a> ReadCursor<'a> {
         self.pos
     }
 
-    /// Perform a transaction on the buffer, returning it to its initial
+    /// Perform a transaction on the cursor, returning it to its initial
     /// state if an error occurs
     #[cfg_attr(feature = "no-panic", no_panic)]
     pub fn transaction<T, R, E>(&mut self, read: T) -> Result<R, E>
     where
         T: FnOnce(&mut ReadCursor<'a>) -> Result<R, E>,
     {
-        let start = self.pos;
+        let start = *self;
         let result = read(self);
-        // if an error occurs, rollback to the starting position
+        // if an error occurs, rollback to the starting state
         if result.is_err() {
-            self.pos = start;
+            *self = start;
         }
         result
     }
@@ -443,4 +443,19 @@ mod tests {
         assert!(field(&mut cursor).is_err());
         assert_eq!(cursor.position(), 3);
     }
+
+    #[test]
+    fn transaction_restores_cursor_replaced_by_callback() {
+        // issue #16: the callback can replace the cursor, not just advance it
+        let mut cursor = ReadCursor::new(&[0x01, 0x02, 0x03]);
+        let result: Result<(), ReadError> = cursor.transaction(|cur| {
+            cur.read_bytes(2)?;
+            *cur = ReadCursor::new(&[]);
+            Err(ReadError)
+        });
+
+        assert!(result.is_err());
+        assert_eq!(cursor.position(), 0);
+        assert_eq!(cursor.read_all(), &[0x01, 0x02, 0x03]);
+    }
 }

src/write.rs

@@ -75,6 +75,9 @@ impl<'a> WriteCursor<'a> {
 
     /// Perform a write transaction which returns the cursor to the original
     /// position if an error occurs
+    ///
+    /// Only the position is restored. Bytes written before the error stay in
+    /// the buffer, past the restored position, and are overwritten by later writes.
     #[cfg_attr(feature = "no-panic", no_panic)]
     pub fn transaction<T, R>(&mut self, write: T) -> Result<R, WriteError>
     where
@@ -92,6 +95,9 @@ impl<'a> WriteCursor<'a> {
     /// Perform a write transaction at particular position. The cursor is always
     /// returned to its original position regardless of the success or failure of
     /// the operation
+    ///
+    /// Only the position is restored. If the operation fails partway, the bytes it
+    /// already wrote stay changed, including any before the original position.
     #[cfg_attr(feature = "no-panic", no_panic)]
     pub fn at_pos<T, R>(&mut self, pos: usize, write: T) -> Result<R, WriteError>
     where
@@ -112,6 +118,8 @@ impl<'a> WriteCursor<'a> {
     }
 
     /// Return the data that has been written since a particular write position
+    ///
+    /// Returns [`WriteError::NumericOverflow`] if `pos` is past the current position.
     #[cfg_attr(feature = "no-panic", no_panic)]
     pub fn written_since(&'a self, pos: usize) -> Result<&'a [u8], WriteError> {
         match self.dest.get(pos..self.pos) {