Microcontroller ringbuffer

Question

I'm pretty new to microcontrollers and C in general, so I thought asking here would be a good way not to get started with bad habits.

/*
 * ringbuffer.h
 *
 *  Created on: Jun 11, 2014
 *      Author: Silly Freak
 */

#ifndef RINGBUFFER_H_
#define RINGBUFFER_H_

//for size_t
//TODO is it a good idea to force size_t for head and available?
#include <stdlib.h>

//defines a buffer type with the given element type and size
//usage: <modifier> RBUF(type, size) rbufname;
#define RBUF(TYPE, SIZE)                                            \
    struct {                                                        \
        TYPE buf[SIZE];                                             \
        size_t head, available;                                     \
    }

//initializes the buffer as being empty
#define RBUF_INIT(NAME)                                             \
    do {                                                            \
        (NAME).head = 0;                                            \
        (NAME).available = 0;                                       \
    } while(0)

//number of slots in the buffer, i.e. buffer size/element size
#define RBUF_SIZE(NAME)                                             \
    (sizeof((NAME).buf)/sizeof((NAME).buf[0]))


//number of filled slots in the buffer
#define RBUF_AVAILABLE(NAME)                                        \
    ((NAME).available)

//number of free slots in the buffer
#define RBUF_FREE(NAME)                                             \
    (RBUF_SIZE(NAME) - RBUF_AVAILABLE(NAME))


//index of the reading head into the ring buffer
#define RBUF_RHEAD(NAME)                                            \
    ((NAME).head)

//index of the writing head into the ring buffer
#define RBUF_WHEAD(NAME)                                            \
    ((RBUF_RHEAD(NAME) + RBUF_AVAILABLE(NAME)) % RBUF_SIZE(NAME))

//pushes an entry into the ring buffer
//the client has to ensure that RBUF_FREE > 0
#define RBUF_PUSH(NAME, VALUE)                                      \
    do {                                                            \
        (NAME).buf[RBUF_WHEAD(NAME)] = VALUE;                       \
        (NAME).available++;                                         \
    } while(0)

//pops an entry out of the ring buffer
//the client has to ensure that RBUF_AVAILABLE > 0
#define RBUF_POP(NAME, VAR)                                         \
    do {                                                            \
        VAR = (NAME).buf[RBUF_RHEAD(NAME)];                         \
        (NAME).head = ((NAME.head) + 1) % RBUF_SIZE(NAME);          \
        (NAME).available--;                                         \
    } while(0)

#endif /* RINGBUFFER_H_ */

/*
 * main.c
 *
 *  Created on: Jun 11, 2014
 *      Author: Silly Freak
 */

#include <inttypes.h>

#include "ringbuffer.h"

RBUF(uint8_t, 10) ringbuf1;

int main(void) {
    RBUF_INIT(ringbuf1);
    if(RBUF_FREE(ringbuf1) > 0) {
        RBUF_PUSH(ringbuf1, 1);
    }
    if(RBUF_AVAILABLE(ringbuf1) > 0) {
        uint8_t var;
        RBUF_POP(ringbuf1, var);
    }
    return 0;
}

Well, it's a ringbuffer. A canonical implementation would probably be this, but targeting microcontrollers, I wanted to avoid having to use a dynamic size while still maintaining reusability for different sizes. (A tutor at my university told me that having to dereference pointers for things like rb->buf[(rb->head + rb->available) % rb->size] could get me point deductions, so I thought it might matter)

Questions:

• Was this decision (likely) worth it, performance wise?
• Regarding the TODO (~ line 10), should I
  • leave it as it is,
  • make the user choose a type that fits his SIZE,
  • or make the compiler choose a type of appropriate size (is this possible?)
• Are there any offenses, style-wise? Formatting, naming conventions, comments/documentation, includes?
• Is my use of macros here considered abuse? Should/could they be formulated as inline functions?
• Is my use of macro parameters robust? For example, in RBUF_POP, I wrote VAR = instead of (VAR) =. Are there situations where this does not work? Would (VAR) = universally work?
• Any other things that would make this code better C?

Answer 1

As for your TODO related question, yes, absolutely, if you use size_t as part of your code's interface, you should #include the relevant definition for it, so what you've got is correct as it stands. However, there are some things here that I think could be improved.

Reduce use of macros

The problem with the overuse of macros like this is that they lead to limitations. For example, let's say I want to write a print routine to print out the contents of a buffer.

void printbuff(something *buff);

What should that something be? Because of the way your RBUF macro is defined, the struct is anonymous and therefore there is no way to make reference to it. For that same reason, there's no easy way to create an array of ring buffers or to include one in a structure.

The other reason not to abuse macros like this is code bloat. If you use RBUF_PUSH at five hundred locations in your code, you'll create five hundred copies of that code. That's not good for embedded systems which are often resource constrained.

available is a misleading variable name

I would have expected structure member variable available to keep a count of the available slots in the ring buffer. Instead, it actually keeps a count of the used slots. Even worse is that the comment and the actual operation of RBUF_AVAILABLE directly contradict each other.

Avoid the use of the % operator

Especially in embedded systems, the % operator is computationally costly and often of variable execution time. For both of those reasons, it's best avoided. The usual way to do this on small microcontrollers is to have the ring buffer size equal to some power of 2 and then use the bitwise & operator to wrap the buffer around.

Think about data alignment

On some processors, access to misaligned data causes a fault; on others it just slows things down. Because your struct is defined with the data buffer first, it means that an odd SIZE buffer may (depending on how your compiler packs structure members) put both head and available at odd addresses even if the struct itself is aligned. For that reason, it is probably better to place those ahead of the buffer instead.

Return a value from RBUF_POP

Instead of requiring the passing of an lvalue into your RBUF_POP macro, consider instead simply returning the value. This would allow constructs such as this:

switch(RBUF_POP(ringbuf1)) 
{
case 'a':
/* etc... */
}

Think of your user

It makes sense that your interface doesn't (presumably for performance reasons) force the user to call the various bounds checks for every push or pop, but it would be nice to have provided a convenience function which would do so, removing some of the burden from the user of your buffer.