From 511f1727600a518bc6bea2582ff3431be022b67f Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Thu, 15 Oct 2026 12:44:21 -0700 Subject: [PATCH 1/2] Restore the whole cursor when a read transaction fails Only the position was restored, so a callback that replaced the cursor and then failed left it pointing into the replacement input. ReadCursor is Copy, so save and restore all of it. Fixes #16 --- src/read.rs | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/read.rs b/src/read.rs index e3da5d9..6d782fe 100644 --- a/src/read.rs +++ b/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(&mut self, read: T) -> Result where T: FnOnce(&mut ReadCursor<'a>) -> Result, { - 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]); + } } From 95b571e868807a969232ac9f209f75973c5e3af2 Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Thu, 15 Oct 2026 12:44:21 -0700 Subject: [PATCH 2/2] Clarify write transaction and error docs Write transactions restore only the position, not buffer contents; say so, including that a failed at_pos can leave bytes before the original position changed. Document the error written_since returns, and correct the TrailingBytes::count description. --- CHANGELOG.md | 2 ++ src/write.rs | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f67831..c585cb5 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/src/write.rs b/src/write.rs index c27775c..90a0222 100644 --- a/src/write.rs +++ b/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(&mut self, write: T) -> Result 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(&mut self, pos: usize, write: T) -> Result 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) {