The proper way to initialize a list struct with resizable array

Question

I have an initialization for a struct that I keep getting a segmentation fault: 11 for.

typedef struct {
    int id;
    double value;
    char* name;
} Item;

typedef struct {
    Item** items;
    int length;
    int capacity;
} List;

List* initList()
{
    List* list = NULL;
    list->items = (Item**)malloc(10 * sizeof(Item*));
    list->length = 0;
    list->capacity = 10;
    return list;
}

Here in main.c is where I get an error per GDB:

List* list = initList();

I cannot initialize, I notice because there is a problem with

list->items = (Item**)malloc(10 * sizeof(Item*));

I am new to C, but I am sure that my initList function will create a list with the given assignments. I also have a respective function to free the items in initList.

`List* list = NULL;` you don't allocate a `List` structure, but you're filling it in the next line. Undefined behaviour. — Jean-François Fabre, Oct 23 '17 at 20:05
`List* list = NULL; list->items = ...`. You are dereferencing `list` which is NULL. — MFisherKDX, Oct 23 '17 at 20:05
After stating `List* list = NULL;` the next line `list->items = ...` is derefencing a `NULL` pointer. — Weather Vane, Oct 23 '17 at 20:06

dbush · Accepted Answer · 2017-10-23T20:07:55.393

The problem is here:

List* list = NULL;
list->items = (Item**)malloc(10 * sizeof(Item*));

You initialize list with a NULL value, then you dereference list without giving it another value. Dereferencing a NULL pointer invokes undefined behavior.

You need to allocate memory for list. Then you can dereference it:

List* list = malloc(sizeof(List *));
if (!list) {
    perror("malloc failed");
    exit(1);
}
list->items = malloc(10 * sizeof(Item*));
if (!list->items) {
    perror("malloc failed");
    exit(1);
}

Also, note that you shouldn't cast the return value of malloc, and don't forget to check for errors.

Jean-François Fabre · Answer 2 · 2017-10-23T20:11:07.123

in

list->items = (Item**)malloc(10 * sizeof(Item*));

the right hand side isn't the issue. The issue is that because list points to NULL, assigning something to one of it's field is undefined behaviour.

You could avoid this allocation, by returning a structure instead of a pointer. No need to allocate the structure dynamically, it has a fixed size.

List initList()
{
    List list;
    list.items = malloc(10 * sizeof(Item*));
    list.length = 0;
    list.capacity = 10;
    return list;
}

caller:

List list = initList();

the copy overhead is very small, but the benefits of not carrying another dynamically allocated pointer for nothing is big.

Is `List list = NULL;` legal? I've always used `memset` – MFisherKDX Oct 23 '17 at 20:10 — MFisherKDX, Oct 23 '17 at 20:10

The proper way to initialize a list struct with resizable array

2 Answers2