C dynamic array implementation using preprocessor

Question

I have implemented a generic dynamic array implementation using defines. As I am new to C, I am not sure whether the code has any flaws. The code is as follows,

#define INIT_CAPAC 5
#define INC_CAPAC 5
#define list(t) struct{ t* data; unsigned int count; unsigned int capac;} 
#define list_init(lst) lst.data = malloc(sizeof(*lst.data) * INIT_CAPAC); \
 lst.count = 0; \
 lst.capac = INIT_CAPAC;
#define list_append(lst, item) lst.data[lst.count++] = item; \
 if(lst.count >= lst.capac) { \
 realloc(lst.data, sizeof(*lst.data) * (lst.capac + INC_CAPAC)); \
 } 
#define list_count(lst) lst.count
#define list_at(lst, index) lst.data[index] 
typedef list(int) int_list_t;
int main() {
 int_list_t arr;
 list_init(arr);
 list_append(arr, 10);
 list_append(arr, 20);
 list_append(arr, 30);
 printf("Count: %d\n", list_count(arr));
 printf("Item at 0: %d\n", list_at(arr, 0));
 printf("Item at 1: %d\n", list_at(arr, 1)); 
 printf("Item at 2: %d\n", list_at(arr, 2));
}

This implementation seems better than other C solutions because,

• We don't store the item size as the part of array structure. In other dynamic array implementations, the size of each element is stored in along with the element count and array capacity.

• This is as intuitive as the dynamic arrays in other high level languages like C++, C#.

Answer 1

list_init and list_append aren't safe. For example:

if (something)
 list_init(arr, 42);

Will be pre-processed to:

if (something)
 arr.data = malloc(...);
arr.count = 0; // BUG - this is unconditional when it shouldn't be
arr.capac = ...; 

The usual fix for this is to wrap the macros is a do { ... } while (0) block.

list_append appears to be buggy: lst.capac is never updated.

Also if your users get a bit fancy and use pointers to lists (or say an array of X lists), they'll get problems:

// contrived example
int_list_t lists[10];
int_list_t *iter = &(lists[0]);
for (int i=0; i<10; i++) {
 list_init(*iter++);
}

This will fail to compile, and if it did compile directly (with added parens), it would evaluate the ++ several times each call - not good. To protect against that you need to somehow only evaluate lst once - usually done by creating a temporary in the do { } while (0) block, but since you don't have the type information, that's not doable in standard C (as far as I know). Can be done with GCC and clang with the typeof extension though.

Answer 2

Here are some things that may help you improve your code.

Use the appropriate #includes

This program requires two headers, both of which should be included:

#include <stdio.h>
#include <stdlib.h>

Check the return value of malloc

If the program runs out of memory, a call to malloc (or realloc) can fail. The indication for this is that the call will return a NULL pointer. You should check for this and avoid dereferencing a NULL pointer (which typically causes a program crash).

Don't leak memory

This code leaks memory. To begin with, the code calls malloc but there is no corresponding call to free. For a usable general list, there should be something akin to list_destroy().

Be careful with multiline macros

Multiline macros can have odd effects if you're not careful. For instance, if we do this:

for (int i=10; i < 70; ++i)
 list_append(arr, i);

The effect is that the if part of list_append is only executed once rather than once per iteration. This can be fixed by wrapping all multiline macros in their own block by enclosing them in curly braces. This will, however, have the side effect of no longer requiring a terminating semicolon if that matters to you.

• scottbb: As mentioned by @Mat, for multi-line function-like macros, the normal idiom is to enclose the macro statements in do { ... } while (0). This requires the trailing semicolon, as well as being safe to use between a single-line conditional and its else statement. Just surrounding the macro definition in curly braces will cause problems following a single-line if (), if there is an else present.