Show HN: Embedded Ring Buffer C++98 (opens in new tab)

Ouch, yeah. They need std::fill(). This is the kind of thing that makes you lose faith in a C++ library. (Also, what's with the volatile private variables?!)

N.B. I'm writing my comments as I am reading your code. Please, don't take offense from my criticism, it is meant to be constructive, albeit concise.

/* * @brief Retrieve a continuous block of * valid buffered data. * @param num_reads_requested How many reads are required. * @param skip Whether to increment the read position * automatically, (false for manual skip * control) * @return A block of items containing the maximum * number the buffer can provide at this time. / Block<T> Read(unsigned int num_reads_requested)

Where is skip?

> if (buffer_full) > { > /

> * Tried to append a value while the buffer is full. > / > overrun_flag = true; > } > else > { > / > * Buffer isn't full yet, write to the curr write position > * and increment it by 1. > / > overrun_flag = false; > data[write_position] = value; > write_position = (write_position + 1U) % LENGTH; > }

You don't write in the case of an "overrun". Isn't that one of the most interesting property of a ringbuffer? It seems that your implementation is specific to your use case (buffering to sd cards?). I don't think your _current_ implementation is apropriate for a _general_ purpose ring buffer mostly because of api concerns. It may be interesting to emphasis this part in your doc.

> reads_to_end = LENGTH - read_position; > req_surpasses_buffer_end = num_reads_requested > reads_to_end; > > if (req_surpasses_buffer_end) > { > /

> * If the block requested exceeds the buffer end. Then > * return a block that reaches the end and no more. > / > block.SetStart(&(data[read_position])); > block.SetLength(reads_to_end); > } > else > { > / > * If the block requested does not exceed 0 > * then return a block that reaches the number of reads required. > */ > block.SetStart(&(data[read_position])); > block.SetLength(num_reads_requested); > }

Maybe : > reads_to_end = LENGTH - read_position; > eff_reads = (num_reads_requested > reads_to_end) ? reads_to_end : num_reads_requested; > //or : eff_reads = std::min(num_reads_requested, reads_to_end) > block.SetStart(&(data[read_position])); > block.SetLength(eff_reads);

Still, I understand the need to be very explicit in an embedded context.

Same principle for ``if (!bridges_zero)`` (the ``else`` case)

Looks reasonable! On a system with virtual memory, one popular trick is the "virtual ring buffer," which lets the reader and writer always access a contiguous region of the full requested length. The idea is to map the same backing store twice sequentially in virtual memory. It leads to a much simpler implementation, because you don't have to handle the edge cases that relate to wrapping around.

Sample implementation in C++17 here:

https://github.com/stanford-stagecast/audio/blob/main/src/ut...

https://github.com/stanford-stagecast/audio/blob/main/src/ut...

One thing I found useful is instead of

  volatile T member;

  T access( void ) { return member; }

Use volatile on the functions instead:

  T member;

  T access( void ) volatile { return member; }

This has causes all access to this-> within the function to be volatile, including read_position, write_position, overrun_flag and data[].

When inlining these functions within other parts of the code, the compiler can hoist reads into registers from the non-volatile members unless you add the compiler barrier that @ghhhhhk8899jj mentioned.

Eh. It's fine in my experience... of all the things I've gotten bitten by in C++, this has not been one of them. It's actually a bit more readable IMO... nullptr doesn't scream "null" like it should. And it's best not to overload int with pointers anyway, because other people using your code may still use NULL even if you personally don't. The one thing to watch out for is usage in template call sites, where you'd want to cast it to the correct type first, but at that point you'd want to cast the nullptr too.

What else then? In c++11 you would of course use nullptr_t, but the OP code is pre-c++11.

GCC used to use a magic builtin in pre C++11 to implement NULL, but IIRC they removed it as non-conforming.

From the article:

> Are there any drawbacks to using nullptr instead of NULL? No, unless you target old compilers that don’t support C++11, which is very unlikely.

and OP is specifically targeting c++98.

Right, I'm using the most recent C++ standard via GCC on STM32 parts now.

exactly. The fence is required even when communicating with a signal handler (or interrupt handler) on the same thread. For for the multithreaded case, depending on the architecture, the compiler barrier is likely not sufficient and an actual hardware #StoreStore fence might be required.

A specular barrier is needed on the reader side.