Multiple producer single consumer with Circular Buffer

Question

Need help in getting the following to work.

I have a multiple producer threads (each writing say 100 bytes of data) to ringbuffer. And one single reader(consumer) thread ,reads 100 bytes at a time and writes to stdout.(Finally i want to write to files based on the data)

With this implementation ,I get the data read from ring buffer wrong sometimes. see below Since the ringbuffer size is small it becomes full and some part of data is loss.This is not my current problem.

** Questions:

On printing the data thats read from ringbuffer ,some data gets interchanged !!I'm unable to find the bug.
Is the logic/approach correct ? (or) Is there a better way to do this

ringbuffer.h

#define RING_BUFFER_SIZE  500
struct ringbuffer
{
    char *buffer;
    int wr_pointer;
    int rd_pointer;
    int size;
    int fill_count;
};

ringbuffer.c

#include <stdio.h>
#include <stdlib.h> 
#include <string.h>
#include "ringbuffer.h"

int init_ringbuffer(char *rbuffer, struct ringbuffer *rb, size_t size)
{
    rb->buffer = rbuffer;
    rb->size = size;
        rb->rd_pointer = 0;
        rb->wr_pointer = 0; 
        rb->fill_count = 0;
    return 0;
}

int rb_get_free_space (struct ringbuffer *rb)
{ 
    return (rb->size -  rb->fill_count);
}

int rb_write (struct ringbuffer *rb, unsigned char * buf, int len)
{
    int availableSpace;
    int i;

    availableSpace = rb_get_free_space(rb);
    printf("In Write AVAIL SPC=%d\n",availableSpace);
    /* Check if Ring Buffer is FULL */
    if(len > availableSpace)
    {
       printf("NO SPACE TO WRITE - RETURN\n");
       return -1;
    }

    i = rb->wr_pointer;
    if(i == rb->size) //At the end of Buffer 
    {
       i = 0;
    }    
    else if (i + len > rb->size)
    {
        memcpy(rb->buffer + i, buf, rb->size - i);
        buf += rb->size - i;
        len = len - (rb->size - i);
        rb->fill_count += len;
        i = 0;
    }
    memcpy(rb->buffer + i, buf, len);
    rb->wr_pointer = i + len;
    rb->fill_count += len;

    printf("w...rb->write=%tx\n", rb->wr_pointer );
    printf("w...rb->read=%tx\n", rb->rd_pointer );
    printf("w...rb->fill_count=%d\n", rb->fill_count );
    return 0;
}

int rb_read (struct ringbuffer *rb, unsigned char * buf, int max)
{
    int i;

    printf("In Read,Current DATA size in RB=%d\n",rb->fill_count);
    /* Check if Ring Buffer is EMPTY */
    if(max > rb->fill_count) 
    {
      printf("In Read, RB EMPTY - RETURN\n");
      return  -1; 
    }  

    i = rb->rd_pointer;
    if (i == rb->size)
    {
       i = 0;
    }
    else if(i + max > rb->size)
    {
        memcpy(buf, rb->buffer + i, rb->size - i);
        buf += rb->size - i;
        max = max - (rb->size - i);
        rb->fill_count -= max;
        i = 0;
    }
    memcpy(buf, rb->buffer + i, max);
    rb->rd_pointer = i + max;
    rb->fill_count -= max;

    printf("r...rb->write=%tx\n", rb->wr_pointer );
    printf("r...rb->read=%tx\n", rb->rd_pointer );
    printf("DATA READ ---> %s\n",(char *)buf);
    printf("r...rb->fill_count=%d\n", rb->fill_count );
    return 0;
}

Serge Serge · Accepted Answer · 2012-10-25T14:54:14

At the producer you also need to wait on conditional variable for the has empty space condition. The both conditional variables should be signaled unconditionally, i.e. when a consumer removes an element from the ring buffer it should signal the producers; when a producer put something in the buffer it should signal the consumers. Also, I would move this waiting/signaling logic into rb_read and rb_write implementations, so your ring buffer is a 'complete to use solution' for the rest of your program.

Multiple producer single consumer with Circular Buffer

3 Answers