Circular RingBuffer

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 ?

-
RB_pop does not check for an empty buffer and has no way of indicating that the buffer is empty. –  William Morris Oct 12 '12 at 14:24
I would consider it better style to use size_t instead of int64_t for all sizes and counts of in-memory objects. –  Seg Fault Oct 27 '12 at 7:11

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.

-

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...
}

-
The point of RB_free is encapsulation, and replacing it with plain free would defeat that. He likely does not intend for consumers of the data type to operate on the struct directly but rather use the interface exposed via the functions. That allows him to alter how the internals work without worry about breaking consuming code. If he makes the end user call free explicitly, then suddenly this is ruined. What if he were to add a second buffer? Suddenly every free() needs a new one. Using a function to handle it means just changing one place, encapsulating that functionality. –  Corbin May 30 at 5:23
@Corbin: Ah, okay. Thanks. I wasn't aware that that was the intent here, also considering that this is C. I'll remove it. –  Jamal May 30 at 5:32