C Basic ring buffer issue

Question

I'm having an issue with a basic ring buffer that I am trying to make. I can not seem to get it to wrap around correctly. When I hit the number of elements = buffer_length, the behavior becomes unpredictable and in the debugger it will only show the first * elements.

Can anyone see a glaring error that I am missing.

Thanks In advance.

#include "stdafx.h"
#include <stdio.h>
#include <string.h>

#define BUFFER_LENGTH 8

typedef struct circular_buffer
{
    float buffer[BUFFER_LENGTH];
    float *buffer_end;
    float *head;
    float *tail;
    size_t count;
} circular_buffer;

circular_buffer buffer;

void cb_init(circular_buffer *cb)
{
    memset(cb->buffer, 0, BUFFER_LENGTH * sizeof(float));

    cb->buffer_end = (float *)cb->buffer + BUFFER_LENGTH * sizeof(float);
    cb->count = 0;
    cb->head = cb->buffer;
    cb->tail = cb->buffer;
}

void cb_push_back(circular_buffer *cb, float item)
{
    *cb->head = item;
    cb->head = (float *)cb->head++;
    if (cb->head == cb->buffer_end)
        cb->head = &cb->buffer[0];
    cb->count++;
}

float cb_pop_front(circular_buffer *cb)
{
    float item = 0;
    item = *cb->tail;
    cb->tail = (float*)cb->tail++;
    if (cb->tail == cb->buffer_end)
        cb->tail = &cb->buffer[0];
    cb->count--;
    return item;
}

float cb_peek_front(circular_buffer *cb, size_t pos)
{
    float *arrPos;
    size_t i = 0;
    do
    {
        arrPos = (float*)cb->tail + i;
        if (arrPos == cb->buffer_end)
            arrPos = cb->buffer;
        i++;
    } while (i <= pos);

    return *arrPos;

}

int _tmain(int argc, _TCHAR* argv[])
{
    cb_init(&buffer);

    cb_push_back(&buffer, 5);
    printf("%f\n",cb_pop_front(&buffer));
    cb_push_back(&buffer, 6);
    printf("%f\n", cb_pop_front(&buffer));
    cb_push_back(&buffer, 7);
    printf("%f\n", cb_pop_front(&buffer));
    cb_push_back(&buffer, 8);
    printf("%f\n", cb_pop_front(&buffer));
    cb_push_back(&buffer, 9);
    printf("%f\n", cb_pop_front(&buffer));
    cb_push_back(&buffer, 10);
    printf("%f\n", cb_pop_front(&buffer));
    cb_push_back(&buffer, 11);
    printf("%f\n", cb_pop_front(&buffer));
    cb_push_back(&buffer, 12);
    printf("%f\n", cb_pop_front(&buffer));

    // stops working here
    cb_push_back(&buffer, 13);
    printf("%f\n", cb_pop_front(&buffer));

    cb_push_back(&buffer, 14);
    printf("%f\n", cb_pop_front(&buffer));


    return 0;
}

Why don't you just treat the buffer as an array and work with an index instead of a pointer? Then the index wraps with say `index = (index + 1) % BUFFER_LENGTH` when advancing forward, or `index = (index + BUFFER_LENGTH - 1) % BUFFER_LENGTH` when moving backwards. — Weather Vane, Mar 08 '16 at 14:17
You don't test for "buffer full" on input or "buffer underrun" on output. The only test you do there is for "buffer run-around". Also, having "head", "tail" *and* "count" is somewhat redundant. — tofro, Mar 08 '16 at 14:17
`cb->head = (float *)cb->head++;` Undefined behavior here. And why convert from float to float? — Lundin, Mar 08 '16 at 14:43

alk · Accepted Answer · 2016-03-08T14:38:31.053

1

This

cb->buffer_end = (float *)cb->buffer + BUFFER_LENGTH * sizeof(float);

will not do what you expect.

Change it to be

cb->buffer_end = cb->buffer + BUFFER_LENGTH;

to have it point just beyond to the buffer's last element.

Also just remove all those useless casts to (float *).

Moreover you want to replace

cb->head = (float *)cb->head++;

by just a simple

cb->head++;

and

cb->tail = (float*)cb->tail++;

by

cb->tail++;

edited Mar 08 '16 at 14:38

answered Mar 08 '16 at 14:12

alk

69,737
10
105
255

1

seems adequate to fix the problem in OP's code. It will be great if the downvoter left a comment. – Manos Nikolaidis Mar 08 '16 at 14:30
@ManosNikolaidis, not me, but the downvote was on the first draft of the answer, which was then temporarily deleted. – Weather Vane Mar 08 '16 at 14:37

score 1 · Answer 2 · answered Mar 08 '16 at 14:20

You are incorrctly considering the size of the element when using pointer arithmetic. For example

bufpointer += 1;

advances to the next element, exactly like you do with array indexing, where you also do not need to consider the size of each element. So

cb->buffer_end = (float *)cb->buffer + BUFFER_LENGTH * sizeof(float);

becomes

cb->buffer_end = cb->buffer + BUFFER_LENGTH;

C Basic ring buffer issue

2 Answers2