0

I'm working on a high-reliance implementation of an algorithm for an embedded system.

in main.c:

    //.. in main()
    int queue_buffer[QUEUE_LEN + 1] = { 0 };
    Queue queue;
    queue_init(&queue, QUEUE_LEN, queue_buffer);
    do_things_on_queue(&queue);
    //.. in main()

in queue.c:

void queue_init(Queue *q, int len, int *data) {
    q->head = 0;
    q->tail = 0;
    q->len = len;
    q->data = data; // an array of length `len + 1`
}

in queue.h:

typedef struct queue {
    int head;
    int tail;
    int len;
    int *data;
} Queue;

I would like to 1. have main.c to not know about Queue; and 2. not use malloc for intializing queue_buffer_ but rather do it statically.

this implies that ideally main.c would be:

    //.. in some function
    Queue *queue = queue_init(something_eventually);
    do_things_with_queue(queue);
    //.. in some function

Is it possible to modify queue_init in queue.cto achieve this in C99? If so, what's the best approach?


Tentative Solutions

I am aware of the technique discussed in this post yet they seems unfeasible without using malloc. I know for sure that I will simultaneously have 4 queues at most. This makes me think that I could declare a memory pool for the queues as a static global array of queues of size 4. Is it ok to use global variables in this case?

@KamilKuk suggested to just have queue_init to return the structure itself since QUEUE_LEN is known at compile time. This requires the following:

in queue.c:

Queue queue_init(void) {
    Queue q;
    
    q.head = 0;
    q.tail = 0;
    q.len = QUEUE_LEN;
    for (int i=0; i < QUEUE_LEN; i++)
        q.data[i] = 0;
    return q;
}

in queue.h:

typedef struct queue {
    int head;
    int tail;
    int len;
    int data[QUEUE_LEN];
} Queue;

Queue queue_init(void);

This appears to greatly simplify the structure initialization. However this does not solve the privacy problem, since main.c should know about Queue to initialize this struct.


Thank you.

deppep
  • 135
  • 1
  • 1
  • 7
  • 1
    If you have only one instance of `Queue`, hide it in `queue.c` and provide only the API working on that specific instance. – Eugene Sh. Feb 07 '23 at 19:25
  • @EugeneSh. I have 4 instance at most of queue. – deppep Feb 07 '23 at 19:26
  • 1
    Then hide all four. As long as you have some fixed number, you can hide them. You will need to reference them somehow though (by ordinal or something) – Eugene Sh. Feb 07 '23 at 19:27
  • @EugeneSh. I appreciate a lot Eugene. I don't know how to do that. should I declare a global static variable for a "memory pool" in `queue.c`? – deppep Feb 07 '23 at 19:30
  • 2
    Since you want to initialize the queues statically, it implies that you have some fixed capacity. So just have static buffers allocated for each queue and that's it. – Eugene Sh. Feb 07 '23 at 19:32
  • @EugeneSh. so say i want 4 queue at most. you mean that i should define 4 a global static arrays in` queue.c` ? – deppep Feb 07 '23 at 19:34
  • 1
    I do not understand, you want to _statically_ initialize queue, _or_ have "in some function" the code `Queue queue = queue_init`. Just return the queue from queue init. `not use malloc for intializing queue_buffer_` The code you presented does not do that. `but rather do it statically` The code you are showing below is "in some function", it can't be static. – KamilCuk Feb 07 '23 at 20:35
  • @KamilCuk thank you! That would be ideal but it can't make it work. I have edited the op. – deppep Feb 07 '23 at 21:17
  • @KamilCuk ohhh I think I got what you are suggesting!! I was declaring my struct wrong! If you provide an answer I will mark it as correct. thank you again. – deppep Feb 07 '23 at 21:24
  • 1
    I do not know what I am suggesting. If all queues are constant, just make the buffer part of the structure. I believe you may consider researching some topics in C, like lifetime of objects depending on storage duration or compound literals. – KamilCuk Feb 07 '23 at 21:25
  • @KamilCuk Yes, i was defining the struct wrong. changing the struct definition to contain an array member `int data[QUEUE_LEN];` everything seems to work. I will research the topics you mention. Thank you again! – deppep Feb 07 '23 at 21:29
  • @KamilCuk after some experiment I realize that doing as you suggest seems to not resolve my first request of making the `Queue` definition private to `queue.c`. – deppep Feb 07 '23 at 21:57
  • 1
    You are asking multiple questions at the same time. You can implement PIMPL idiom without dynamic allocation by synchronizing a structure alignment and size between header and source file, or you can just use an index in an array just like file descriptor are implemented on linux. It is totally not worth the time, especially on embedded systems and especially as you do not want to use dynamic memory. – KamilCuk Feb 07 '23 at 22:15
  • Yes I realize the problems are unrelated. Thank you very much for you time and help Kamil. – deppep Feb 07 '23 at 22:21

3 Answers3

1

I would typically do:

// queue.h
#define QUEUE_INIT(data, len)  { .len = len, .data = data }
#define QUEUE_INIT_ON_STACK(len)  QUEUE_INIT((char[len]){0}, len)

// main.c
static Queue queue = QUEUE_INIT_ON_STACK(QUEUE_LEN  + 1);

As for PIMPL idiom, it's easy to implement with descriptors just like file descriptors in LINUX, especially when the count is static.

// queue.h
typedef Queue int;
void do_things_with_queue(Queue);

// queue.c
struct RealQueue { stuff; };
static struct RealQeueue arr[4] = { stuff };
static struct RealQeueue *get_RealQueue(Queue i) {
     assert(0 <= i && i < sizeof(arr)/sizeof(*arr));
     return &arr[i];
}
void do_things_with_queue(Queue i) {
    struct RealQueue *queue = get_RealQueue(i);
}

// main.c
static Queue queue = 1;
// etc.

Or you can break all hell and synchronize alignment between source and header file:

// queue.h
struct Queue {
    // This has to be adjusted __for each compiler and environment__
    alignas(60) char data[123];
};
 
#define QUEUE_INIT() { 0xAA, 0xBB, etc.. constant precomputed data }

// queue.c
struct RealQeueue { stuff; };
static_assert(alingof(struct RealQueue) == alignof(struct Queue), "");
static_assert(sizeof(struct RealQueue) == sizeof(struct Queue), "");
void do_things_with_queue(Queue *i) {
    struct RealQueue *queue = (struct RealQueue*)i->data;
}
KamilCuk
  • 120,984
  • 8
  • 59
  • 111
1

The best way to do this is to pass a buffer and its size to the init function, exactly as you already have.

It is a very bad idea to worry about calling a function versus having the data fixed at compile time. Both the execution time and code size for a tiny initialization like this is negligible. Making your code interface awkward just to save a few instructions at startup is not just a waste of effort, it makes the code hard to maintain and risks introducing bugs.

There are a number of embedded systems or libraries that provide a macro which declares both the storage array and the control structure in one go and gives them a name which is known only to the library, and then you have to use a macro to generate the name every time you access the item. For an example of this you might look at osMailQDef in CMSIS-OS. I don't really recommend this method though. It is too easy to get wrong, whereas doing it the usual way is easy to read and any reviewer will be able to spot a mistake straight away.

Tom V
  • 4,827
  • 2
  • 5
  • 22
  • Thanks for the interesting link and the suggestions. This is certainly comforting :P! However in my "real" case I have to deal with multiple structure, each comprising different buffers. the result is initialization code taking half a page, while i would just prefer to just supply a few functions to the code's user and make him unable to access the structure's members. is this impossible? KamilCuk suggested in the op comment to return the structures themselves to avoid lengthy initializations. this appears to work yet does not provide a solution to the privacy of the struct. THANK YOU A LOT! – deppep Feb 07 '23 at 22:08
  • 1
    If you need a number of different members in the struct then just pass a single byte array and its size and divide it up in the init function. You could even allow space for the struct itself from the byte array and then return a struct pointer. Just don't forget about alignment. – Tom V Feb 08 '23 at 06:50
1

I would like to 1. have main.c to not know about Queue; and 2. not use malloc for intializing queue_buffer_ but rather do it statically.

this implies that ideally main.c would be:

    //.. in some function
    Queue queue = queue_init(something_eventually);
   do_things_with_queue(&queue);
   //.. in some function

No, your objectives do not imply a solution as described. You cannot declare or use an object of type Queue anywhere that the definition of that type is not visible. That follows directly from the language's rules, but if you want a more meaningful justification then consider that even if main does not access any of the members of Queue, it still needs the definition simply to know how much space to reserve for one.

It's not obvious to me that it serves a useful purpose to make type Queue opaque in main.c (or anywhere), but if that's what you want then in that scope you can forward declare it, never define it, and work only with pointers to it:

typedef struct queue Queue;

// ...

    Queue *queue = queue_init(something_eventually);
    do_things_with_queue(queue);

For that to work without dynamic memory allocation, the pointed-to Queue objects must have static storage duration, but that does not mean that they need to be globals -- either in the sense of being accessible via a name with external linkage, or in the sense of being declared at file scope.

Additionally, you have the option of allocating the data arrays automatically, as in your example code, so as to not tie up that memory in queues when they are not in use. If you prefer, you can wrap that up in a macro or two for a bit of additional ease of use (left as an exercise).

For example,
queue.h

typedef struct queue Queue;

Queue *queue_init(int queue_size, int queue_data[]);
void queue_release(Queue *queue);

queue.c

#include "queue.h"

struct queue {
    int head;
    int tail;
    int len;
    int *data;
};

Queue *queue_init(int queue_len, int queue_data[]) {
    // queue_pool has static storage duration and no linkage
    static Queue queue_pool[4] = {{0}};

    // Find an available Queue, judging by the data pointers

    for (Queue *queue = queue_pool;
            queue < queue_pool + sizeof(queue_pool) / sizeof(*queue_pool);
            queue++) {
        if (queue->data == NULL) {
            // This one will do.  Initialize it and return a pointer to it.
            queue->head = 0;
            queue->tail = 0;
            queue->len = queue_len;
            queue->data = queue_data;

            return queue;
        }
    }

    // no available Queue
    return NULL;
}

void queue_release(Queue *queue) {
    if (queue) {
        queue->data = NULL;
    }
}

main.c

    // ... in some function

    int queue_data[SOME_QUEUE_LENGTH];
    Queue *queue = queue_init(SOME_QUEUE_LENGTH, queue_data);
    do_things_with_queue(queue);
    queue_release(queue);

    // ...

Of course, if you prefer, you can put the queue data directly into the queue structure, as in your tentative solution, and maybe provide a flag there to indicate whether the queue is presently in use. That would relieve users of any need to provide storage, at the cost of tying up the storage for all the elements of all the queues for the whole duration of the program.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • You are right in saying that the "ideal" `main.c` should know about `Queue *` and not `Queue` itself, I will correct the op. I have one question: what benefits brings using a static local variable in place of variables with file scope? Thank you very much for the brilliant solution. – deppep Feb 08 '23 at 00:00
  • @deppep, putting the `Queue` pool in a local variable gives it no linkage: not even other functions in the same translation unit can access these objects directly, even though they know everything there is to know about the `Queue` *type*. That might or might not be desirable. For that to work, however, the variable must be declared `static` to give it static storage duration. The lifetime of the resulting objects is not bounded by an execution of the function; instead, it is the entire duration of a program run. – John Bollinger Feb 08 '23 at 04:52
  • (Note: `static` has different meaning for file-scope declarations than for declarations inside functions.) – John Bollinger Feb 08 '23 at 04:55