HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Circular RingBuffer

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
circularringbufferstackoverflow

Problem

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 ?

Solution

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:

-
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;


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

Code Snippets

end_offset = (end_offset + 1) % size;
int8_t RB_pop(ring_buffer* rb)
{
    if (rb == NULL || rb->buffer == NULL)
        return false;

Context

StackExchange Code Review Q#16468, answer score: 7

Revisions (0)

No revisions yet.