OSDev.org

The Place to Start for Operating System Developers
It is currently Thu Mar 28, 2024 2:18 pm

All times are UTC - 6 hours




Post new topic Reply to topic  [ 4 posts ] 
Author Message
 Post subject: C; ring buffer; is this "doable" approach?
PostPosted: Mon Jan 23, 2023 10:07 am 
Offline
Member
Member

Joined: Fri Apr 08, 2022 3:12 pm
Posts: 54
I needed to create ring buffer for my keyboard driver. While I need only to buffer bytes(chars) I was wondering if I can implement more versatile approach if I need it for something else. I did create this but I'm not sure if I'm not pushing it too much.
Here's the implementation:
Code:
#ifndef HAVE_TYPES_H
#define HAVE_TYPES_H

#include <stdint.h>

#define   RINGBUF32_SIZE      32

#define   DEFINE_RING32(name, type) \
   struct rbuf32 {                   \
      uint8_t ri:5;                 \
      uint8_t ci:5;              \
      type buf[RINGBUF32_SIZE];\
   } rbuf32_t;             \
   typedef struct rbuf32 name;

#define STORE_RING32(rb, item) do {         \
      (rb)->buf[(rb)->ci++] = (item);      \
   } while(0)

#define   FETCH_RING32(rb)    (rb)->buf[(rb)->ri++]

#endif /* ifndef HAVE_TYPES_H */

With test program:
Code:
#include <stdio.h>
#include <string.h>
#include "types.h"

#define   SANITY_CHECK   2048

DEFINE_RING32(dbuf32, int)

void dumpbuf(void* b);

int main() {
   dbuf32 buf;
   dbuf32* pbuf = &buf;   // XXX: due to way macros are defined
   int d,i;

   memset(&buf, 0, sizeof(buf));
   i =0;
   while(1) {
      scanf("%d", &d);
      printf("storing: %d\n", d);

      STORE_RING32(pbuf, d);
      if (i++ > SANITY_CHECK || d == 42) {
         break;
      }
   }
   dumpbuf(pbuf);
   for (i=0; i < 6; i++) {
      printf("fetch: %d\n", FETCH_RING32(pbuf));
   }
   dumpbuf(pbuf);
   return 0;
}

void dumpbuf(void* b) {
   dbuf32* buf = (dbuf32*)b;
   printf("ri: %d, ci: %d\n", buf->ri, buf->ci);
   for(int i =0; i < 32; i++) {
      printf("%d ", buf->buf[i]);
   }
   printf("\n");
}

Is this acceptable form of such definition ? I never used macros like these. I mean, it does compile, it doesn't complain. But I'm just not sure if that's a good idea.


Top
 Profile  
 
 Post subject: Re: C; ring buffer; is this "doable" approach?
PostPosted: Mon Jan 23, 2023 12:30 pm 
Offline
Member
Member
User avatar

Joined: Fri Jun 11, 2021 6:02 am
Posts: 96
Location: Belgium
My preference is to have one macro that generates the struct type as well as corresponding functions, e.g.

Code:
#ifndef HAVE_TYPES_H
#define HAVE_TYPES_H

#include <stdint.h>

#define   RINGBUF32_SIZE      32

#define   DEFINE_RING32(name, type) \
   typedef struct { \
      uint8_t ri:5; \
      uint8_t ci:5; \
      type buf[RINGBUF32_SIZE]; \
   } name; \
   \
   void name##_init(name *rb) { \
      memset(rb, 0, sizeof(*rb)); \
   } \
   \
   void name##_store(name *rb, type item) { \
      (rb)->buf[(rb)->ci++] = item; \
   } \
   \
   type name##_fetch(name *rb) { \
      return (rb)->buf[(rb)->ri++]; \
   }

#endif /* ifndef HAVE_TYPES_H */


Code:
#include <stdio.h>
#include <string.h>
#include "types.h"

#define   SANITY_CHECK   2048

DEFINE_RING32(dbuf32, int)

void dumpbuf(dbuf32 *b);

int main() {
   dbuf32 buf;
   int d, i;

   dbuf32_init(&buf);

   i = 0;
   while(1) {
      scanf("%d", &d);
      printf("storing: %d\n", d);

      dbuf32_store(&buf, d);
      if (i++ > SANITY_CHECK || d == 42) {
         break;
      }
   }
   dumpbuf(&buf);
   for (i = 0; i < 6; i++) {
      printf("fetch: %d\n", dbuf32_fetch(&buf));
   }
   dumpbuf(&buf);
   return 0;
}

void dumpbuf(dbuf32 *buf) {
   printf("ri: %d, ci: %d\n", buf->ri, buf->ci);
   for(int i =0; i < 32; i++) {
      printf("%d ", buf->buf[i]);
   }
   printf("\n");
}


You may want to make the generated functions static or weakly linked, depending on how exactly you will use the macro.

_________________
My OS is Norost B (website, Github, sourcehut)
My filesystem is NRFS (Github, sourcehut)


Top
 Profile  
 
 Post subject: Re: C; ring buffer; is this "doable" approach?
PostPosted: Mon Jan 23, 2023 12:36 pm 
Offline
Member
Member

Joined: Wed Aug 30, 2017 8:24 am
Posts: 1593
Well, in principle I always have a bit of a problem with fixed sizes with no obvious justification. The buffer is 32 elements. Depending on the use case, this may be too much or too little, and does not appear to be tunable. But for the most part, this will work. However, some details:
Code:
#define   DEFINE_RING32(name, type) \
   struct rbuf32 {                   \
      uint8_t ri:5;                 \
      uint8_t ci:5;              \
      type buf[RINGBUF32_SIZE];\
   } rbuf32_t;             \
   typedef struct rbuf32 name;
This creates an instance of "struct rbuf32" named "rbuf32_t". I believe you wanted to make a new type here, although I also fail to see what use those names are given that you do everything else in macros, and those names prevent reuse (if you define two different ring buffers, they will get the same name, and the compiler will complain about that). Also, the base type of a bitfield can only be signed int or unsigned int or int (yielding a signed, unsigned, or implementation-defined bitfield, respectively), everything else is a GCC extension. So putting it all together, I'd go with
Code:
#define DEFINE_RING32(name, type) \
  typedef struct { \
    unsigned ri:5; \
    unsigned ci:5; \
    type buf[32]; \
  } name

That also makes the macro not end in a semicolon, so you can add those to the macro calls again, which will likely be less confusing for readers.

_________________
Carpe diem!


Top
 Profile  
 
 Post subject: Re: C; ring buffer; is this "doable" approach?
PostPosted: Mon Jan 23, 2023 1:22 pm 
Offline
Member
Member

Joined: Fri Apr 08, 2022 3:12 pm
Posts: 54
Thanks both for the input and corrections.
nullplan wrote:
This creates an instance of "struct rbuf32" named "rbuf32_t".
That's incorrect design choice on my side. I like the way you and Demindiro set it.

When it comes to static buffer size I agree, it's not ideal. I don't have memory management set yet. I know I should probably focus on more essential parts of kernel first. I liked the idea of creating custom gdb stub first, even at the very beginning of my mios project. That's why I have to have static buffer size.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 4 posts ] 

All times are UTC - 6 hours


Who is online

Users browsing this forum: No registered users and 27 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group