From 00b016957822cef5ce96f3d4e48cf4e447e2b6f7 Mon Sep 17 00:00:00 2001
From: Jef Driesen <jef@libdivecomputer.org>
Date: Mon, 11 Dec 2023 21:25:54 +0100
Subject: [PATCH] Update the internal state in-place

Some of the internal state is cached in local variables at the start of
the function, and is updated only at the end of the function. But the
contents of the packet buffer is never cached. As a result, the two can
go out of sync when an error occurs and the function returns early.
Trying to restore the original state is pointless if the corresponding
data in the packet buffer is no longer available.

Fixed by removing the local variables and always updating the internal
state in-place to reflect the current state.
---
 src/rbstream.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/rbstream.c b/src/rbstream.c
index e16f872..8e7e17a 100644
--- a/src/rbstream.c
+++ b/src/rbstream.c
@@ -125,43 +125,39 @@ dc_rbstream_read (dc_rbstream_t *rbstream, dc_event_progress_t *progress, unsign
 	if (rbstream == NULL)
 		return DC_STATUS_INVALIDARGS;
 
-	unsigned int address = rbstream->address;
-	unsigned int available = rbstream->available;
-	unsigned int skip = rbstream->skip;
-
 	unsigned int nbytes = 0;
 	unsigned int offset = size;
 	while (nbytes < size) {
-		if (available == 0) {
+		if (rbstream->available == 0) {
 			// Handle the ringbuffer wrap point.
-			if (address == rbstream->begin)
-				address = rbstream->end;
+			if (rbstream->address == rbstream->begin)
+				rbstream->address = rbstream->end;
 
 			// Calculate the packet size.
 			unsigned int len = rbstream->packetsize;
-			if (rbstream->begin + len > address)
-				len = address - rbstream->begin;
-
-			// Move to the begin of the current packet.
-			address -= len;
+			if (rbstream->begin + len > rbstream->address)
+				len = rbstream->address - rbstream->begin;
 
 			// Read the packet into the cache.
-			rc = dc_device_read (rbstream->device, address, rbstream->cache, rbstream->packetsize);
+			rc = dc_device_read (rbstream->device, rbstream->address - len, rbstream->cache, rbstream->packetsize);
 			if (rc != DC_STATUS_SUCCESS)
 				return rc;
 
-			available = len - skip;
-			skip = 0;
+			// Move to the end of the next packet.
+			rbstream->address -= len;
+
+			rbstream->available = len - rbstream->skip;
+			rbstream->skip = 0;
 		}
 
-		unsigned int length = available;
+		unsigned int length = rbstream->available;
 		if (nbytes + length > size)
 			length = size - nbytes;
 
 		offset -= length;
-		available -= length;
+		rbstream->available -= length;
 
-		memcpy (data + offset, rbstream->cache + available, length);
+		memcpy (data + offset, rbstream->cache + rbstream->available, length);
 
 		// Update and emit a progress event.
 		if (progress) {
@@ -172,10 +168,6 @@ dc_rbstream_read (dc_rbstream_t *rbstream, dc_event_progress_t *progress, unsign
 		nbytes += length;
 	}
 
-	rbstream->address = address;
-	rbstream->available = available;
-	rbstream->skip = skip;
-
 	return rc;
 }