Circular RingBuffer

Question

I am trying to get into C stuff, and I thought it would be a good idea to try and implement a circular buffer.

I have defined my struct like this:

typedef struct
{
     int8_t* buffer;
     int8_t* buffer_end;
     int8_t* data_start;
     int8_t* data_end;
     int64_t count;
     int64_t size;
 } ring_buffer;

And the functions:

void RB_init(ring_buffer* rb, int64_t size)
{
    rb->buffer = malloc(sizeof(int8_t) * size);
    rb->buffer_end = rb->buffer + size;
    rb->size = size;
    rb->data_start = rb->buffer;
    rb->data_end = rb->buffer;
    rb->count = 0;
}

void RB_free(ring_buffer* rb)
{
    free(rb->buffer);
}

bool RB_push(ring_buffer* rb, int8_t data)
{
    if (rb == NULL || rb->buffer == NULL)
        return false;

    *rb->data_end = data;
    rb->data_end++;
    if (rb->data_end == rb->buffer_end)
        rb->data_end = rb->buffer;

    if (RB_full(rb)) {
        if ((rb->data_start + 1) == rb->buffer_end)
            rb->data_start = rb->buffer;
        else
            rb->data_start++;
    } else {
        rb->count++;
    }

    return true;
}

int8_t RB_pop(ring_buffer* rb)
{
    if (rb == NULL || rb->buffer == NULL)
        return false;

    int8_t data = *rb->data_start;
    rb->data_start++;
    if (rb->data_start == rb->buffer_end)
        rb->data_start = rb->buffer;
    rb->count--;

    return data;
}

bool RB_full(ring_buffer* rb)
{
    return rb->count == rb->size;
}

I did some testing and it seems to work well. Can you suggest some improvements ?

Answer 1

This looks nice. It is very readable and is probably fast.

Sometimes ring buffer wrap is implemented by using the following kind of remainder stuff and offsets:

end_offset = (end_offset + 1) % size;

But I like your way of doing it without offsets and division.

Some minor findings:

1. NULL pointer checks in RB_pop prevents segfaults, but the caller will get a return value of zero. So the caller won't know if zero is an error or a success result.

   int8_t RB_pop(ring_buffer* rb)
   {
       if (rb == NULL || rb->buffer == NULL)
           return false;
   

2. RB_pop and RB_push both do the check: rb == NULL. Maybe other functions should do it too.

Answer 2

I agree with @User1 about RB_pop() and would like to add on to it:

In order to prevent the function from returning an unexpected return value of false, you should make the function void and have a second parameter data. This will also allow you to return early if the first conditional statement is false.

void RB_pop(ring_buffer* rb, int8_t* data)
{
    if (rb == NULL || rb->buffer == NULL)
        return;

    // update data parameter...
}

Answer 3

you didn't check count in pop function. if buffer is empty you will return garbage and also count will now be negative.