The mix of global buffer and passed-in struct defining the buffer made me sad. So did the lack of atomic increments and decrements (volatile ain't sufficient). Don't get me started on the getchar routine.
This stuff just isn't hard. Sigh.
This stuff just isn't hard. Sigh.
It's for MSP430, and seems to be oriented in single core embedded microcontrollers. Those things have no atomic operations, other than disabling interrupts.
You'd be right for multicore chips or if data type is larger than word size or if data is not aligned.
But in this case, this code is correct for this architecture. All of those things you mentioned are perfectly normal and acceptable when programming microcontrollers.
http://www.simplyembedded.org/tutorials/msp430-architecture/
It's not a great coding practice. Move to a different processor (this happens!) and your code turns into a load-add-store, gets interrupted halfway through, and you scratch your head for a while about the mysterious and flaky FIFO failures.
Been there. At least a gesture towards an atomic operation would be nice, when you have data shared between interrupt and non-interrupt code.
#if !defined(__AVR__) && !defined(__MSP430__)
#warning "Fix this FIFO - ++/-- possibly not atomic!"
#endif
into the header of that thingy then. Save work on small uCs, and be warned when moving it somewhere else.
A:read Ax; (Interrupt: B:read Bx; B:write Bx+1; return); A: write Ax+1 # (wrong!)
could occur. But the only error I can see here is that sometimes put() might fail thinking there is overflow when there's maybe one space left.
Am I missing something?
(Further investigation: This[1] thread suggests you can run into problems if your counter type is larger than the native ALU size, because the interrupt could occur midway through your increment operation. But here the size_t is 16 bits, so that should 't be a problem?)
[1] http://www.embeddedrelated.com/showthread/msp430/20400-1.php
e.g.
buf[volatile_write_ptr] = c
volatile_write_ptr++;
For typical "simple" single processor controllers this will not be an issue, though, and "volatile"+spinlocks/disenable-enable-interrupts will work just fine.
But isn't getchar() at least doing the same as before?
If the ring buffer is empty, ring_buffer_get() won't write to its data argument, and since getchar() passes in -1, that's what it's going to be returning if the buffer was empty. Just like it says it did before using a ring buffer.