From: Mikko Rasa <tdb@tdb.fi>
Date: Mon, 28 Oct 2013 18:28:07 +0000 (+0200)
Subject: Rewrite ringbuffer implementation to be interrupt-safe
X-Git-Url: http://git.tdb.fi/?a=commitdiff_plain;h=a4919e44177de59604d8c088487ede49e3884b35;p=model-railway-devices.git

Rewrite ringbuffer implementation to be interrupt-safe

The earlier version had a race condition with the fill variable, where an
interrupt might execute in the middle of increment/decrement and do the
opposite operation.  Each variable is now written to by only one end of
the buffer, eliminating the problem.
---

diff --git a/common/ringbuffer.h b/common/ringbuffer.h
index 1a03ec5..107227b 100644
--- a/common/ringbuffer.h
+++ b/common/ringbuffer.h
@@ -1,52 +1,67 @@
 #ifndef RINGBUFFER_H_
 #define RINGBUFFER_H_
 
-#define RINGBUFFER(name, size) uint8_t name[(size)+4] = { size, 0, 0, 0 }
+#define RINGBUFFER(name, size) uint8_t name[(size)+4] = { (size)+1, 0, 0 }
+
+static inline uint8_t ringbuffer_size(const uint8_t *buffer)
+{
+	return buffer[0]-1;
+}
 
 static inline uint8_t ringbuffer_fill(const uint8_t *buffer)
 {
-	return buffer[1];
+	const uint8_t size = buffer[0];
+	uint8_t head = buffer[1];
+	uint8_t tail = buffer[2];
+	if(head<tail)
+		head += size;
+	return head-tail;
 }
 
 static inline uint8_t ringbuffer_space(const uint8_t *buffer)
 {
-	return buffer[0]-buffer[1];
+	return ringbuffer_size(buffer)-ringbuffer_fill(buffer);
 }
 
 static inline void ringbuffer_push(uint8_t *buffer, uint8_t value)
 {
 	const uint8_t size = buffer[0];
-	uint8_t head = buffer[2];
-	buffer[4+head++] = value;
+	uint8_t head = buffer[1];
+	buffer[3+head] = value;
+	++head;
 	if(head>=size)
 		head -= size;
-	buffer[2] = head;
-	++buffer[1];
+	buffer[1] = head;
 }
 
 static inline void ringbuffer_blocking_push(uint8_t *buffer, uint8_t value)
 {
-	const volatile uint8_t *fill = buffer+1;
-	while(*fill>=buffer[0]) ;
+	const uint8_t size = buffer[0];
+	uint8_t next_head = buffer[1]+1;
+	if(next_head>=size)
+		next_head -= size;
+	const volatile uint8_t *tail = buffer+2;
+	while(next_head==*tail) ;
 	ringbuffer_push(buffer, value);
 }
 
 static inline uint8_t ringbuffer_pop(uint8_t *buffer)
 {
 	const uint8_t size = buffer[0];
-	uint8_t tail = buffer[3];
-	uint8_t value = buffer[4+tail++];
+	uint8_t tail = buffer[2];
+	uint8_t value = buffer[3+tail];
+	++tail;
 	if(tail>=size)
 		tail -= size;
-	buffer[3] = tail;
-	--buffer[1];
+	buffer[2] = tail;
 	return value;
 }
 
-static inline uint8_t rinbuffer_blocking_pop(uint8_t *buffer)
+static inline uint8_t ringbuffer_blocking_pop(uint8_t *buffer)
 {
-	const volatile uint8_t *fill = buffer+1;
-	while(*fill==0) ;
+	const volatile uint8_t *head = buffer+1;
+	uint8_t tail = buffer[2];
+	while(*head==tail) ;
 	return ringbuffer_pop(buffer);
 }