Allocate array of structs with flexible array member, each member of the same isze

76 Views Asked by At

I'd like to ask about your opinion for a code I've figured out.

Context

I'm implementing a queue as a ring buffer. Each queue member is a struct below:

struct member {
  int size;
  char data[]
};

What's important in my case: for each member, the same amount of data should be allocated.

Why don't just provide array size if it's the same for every element?

I want to use multiple queues with different parameters, and just provide number of elements and max size of element buffer as arguments.

Why don't just have pointer in each entry and malloc individually?

I'd like to improve performance and store everything in one memory block, rather than multiple blocks at multiple addresses

My solution

I've got and idea how to make it work, based on 1 assumption: Member data size must be a multiple of 4 - to avoid any padding that could mess up address calculation

NOTE Macros are set just for example. I don't know the real sizes at the compile time

#define DATA_SIZE 64
#define N_OF_STRUCTS 3

struct entry {
  int size;
  char data[]; // 64
};

struct entry *get_at_index(struct entry *buff, int index) {
  return buff +
         index * (offsetof(struct entry, data) + DATA_SIZE * sizeof(char));
}

int main() {
  struct entry *s0 =
      malloc((sizeof(int) + sizeof(char) * DATA_SIZE) * N_OF_STRUCTS);

  struct entry *s1 = get_at_index(s0, 1);
  struct entry *s2 = get_at_index(s0, 2);

  (s0 + sizeof(int) + DATA_SIZE)->data[0] = 'a';
  (s0 + 2 * (sizeof(int) + DATA_SIZE))->data[0] = 'b';
  assert(s1->data[0] == 'a');
  assert(s2->data[0] == 'b');
  
  printf("%s\n", s1->data);
  printf("%s\n", s2->data);

  return 0;
}

What I'd like to know

I'd like to ask what's your opinion on that.

  • Is there something I didn't consider?
  • Would it work on other architectures?
  • Is this anti-pattern and if so, why?
2

There are 2 best solutions below

6
0___________ On
  struct entry *s0 =
      malloc((sizeof(int) + sizeof(char) * DATA_SIZE) * N_OF_STRUCTS);

Is bad as you do not know how your implementation will store your struct int the memory.

  struct entry *s0 =
      malloc((sizeof(*s0) + DATA_SIZE) * N_OF_STRUCT);

Also pointer arithmetic does not happen on the byte (or char) level. You need to learn more about pointers and pointer arithmetic

Generally, your idea does not make too much sense as you use macro definitions for sizes and it is known at compile time. It would make sense if your datatypes were created at run time (so nothing is known when the code is compiled). It looks like an XY problem to me.

I want to use multiple queues with different parameters, and just provide number of elements and max size of element buffer as arguments.

You cant as the sizes are have to be known during compilation.

I'd like to improve performance and store everything in one memory block, rather than multiple blocks at multiple addresses

How can you know if it will have any effect? Did you profile the application? Remember: enter image description here

To have an ability to create those data runtime not having defined data sizes and queue length you need to store a bit more data.

Example allocating one large chunk of memory:

typedef struct 
{
    size_t queuesize;
    size_t elementsize;
    size_t *sizes;
    unsigned char *data;
    unsigned char storage[];
}queue_t;


typedef struct 
{
    size_t size;
    unsigned char *data;
}entry_t;


queue_t *createQueue(size_t size, size_t datasize)
{
    queue_t *q = malloc(sizeof(*q) + size * (datasize + sizeof(q -> sizes[0])));
    if(q)
    {
        q -> sizes = q -> storage; 
        q -> data = q -> storage + sizeof(q -> sizes[0]) * size;
        memset(q -> sizes, 0, sizeof(q -> sizes[0]) * size);
        q -> queuesize = size;
        q -> elementsize = datasize;
    }
    return q;
}

entry_t get_at_index(queue_t *q, size_t element) 
{
    entry_t t;
    t.size = q -> sizes[element];
    t.data = q -> data + q -> elementsize * element;
    return t;
}

void put_at_index(queue_t *q, size_t element, size_t datasize, void *data) 
{
    q -> sizes[element] = datasize;
    memcpty(q -> data + q -> elementsize * element, data, datasize);
}

0
dbush On

This won't do as you expect:

(s0 + sizeof(int) + DATA_SIZE)->data[0] = 'a'; 
(s0 + 2 * (sizeof(int) + DATA_SIZE))->data[0] = 'b';

Because pointer arithmetic is done in terms of multiples of the size of the pointed-to object. For example s0 + sizeof(int) + DATA_SIZE doesn't advance the pointer by sizeof(int) + DATA_SIZE bytes but by sizeof(struct entry) * (sizeof(int) + DATA_SIZE) bytes. So you won't be accessing the element you think you are.

The same problem exists here:

return buff +
     index * (offsetof(struct entry, data) + DATA_SIZE * sizeof(char));

And has an additional issue, namely that it doesn't account for any padding the might exists between the size and data members.

So there are two main things you need to do here:

  • Use a char * when offsetting the array so pointer arithmetic moves 1 byte at a time.
  • Since sizeof(struct entry) takes into account any padding that might exits between the members but not the flexible array member, your offset just needs to be sizeof(struct entry) * DATA_SIZE;
struct entry *get_at_index(struct entry *buff, int index) {
  return (struct entry *)((char *)buff +
         index * ((sizeof(struct entry) + DATA_SIZE));
}

For alignment issues, if you want to allow DATA_SIZE to not be a multiple of 4, you can add trailing padding:

int PAD = (DATA_SIZE % _Alignof(struct entry)) ? 
              (_Alignof(struct entry) - (DATA_SIZE % _Alignof(struct entry))) : 
              0;

struct entry *get_at_index(struct entry *buff, int index) {
    return (struct entry *)((char *)buff +
           index * ((sizeof(struct entry) + DATA_SIZE + PAD));
}

int main() {
  struct entry *s0 =
      malloc(sizeof(struct entry) + DATA_SIZE + PAD) * N_OF_STRUCTS);
  ...