Search code examples
cpointersstructmallocbus-error

Why does this C struct initialization code produce a bus error?


When designing a game entity system in C, I attempted an "equals-free" initialization approach. I was surprised to see a linter tell me there was a memory leak at the end of my init function, and that my variable ent was never initialized in the following code. It turned out to be right because I got a "bus error":

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

typedef struct {
    int x;
    int y;
} entity_t;

void entity_init(entity_t* ent, int _x, int _y)
{
    ent = malloc(sizeof(*ent));
    ent->x = _x;
    ent->y = _y;
}

int main(void)
{
    entity_t* ent;
    entity_init(ent, 10, 24);
    printf("Entity: x%d y%d", ent->x, ent->y);
    return 0;
}

What I thought the above code would do, was take my empty ent pointer supplied as an argument, tell it to point to some newly allocated memory, and then fill in that memory and everything would be fine. I have no idea what it's really doing to cause a "bus error", am I missing something critical about pointers and malloc?

I vaguely remember seeing something very similar to this done before in some C code (equals-free struct initialization) and I would strongly prefer to use an equals-free initialization style similar to this (broken) code if such a thing is possible in C.


Solution

  • Move the malloc call outside the initialization function:

    #include <stdio.h>
    #include <stdlib.h>
    
    typedef struct {
        int x;
        int y;
    } entity_t;
    
    void entity_init(entity_t* ent, int _x, int _y)
    {
        ent->x = _x;
        ent->y = _y;
    }
    
    int main(void)
    {
        entity_t* ent;
        if(NULL==(ent = malloc(sizeof(*ent))))
            return 1;
        entity_init(ent, 10, 24);
        printf("Entity: x%d y%d", ent->x, ent->y);
        return 0;
    }
    

    You're assigning the pointer to the allocated block to a local variable (ent). That can't affect the ent in main.

    If you wanted to keep the malloc in entity_init, you should use a double pointer, but you should also change the signature to allow for a way to signal malloc failure from entity_init

    int entity_init(entity_t **ent, int _x, int _y)
    {
        if(NULL==(*ent = malloc(sizeof(**ent))))
            return -1;
        (*ent)->x = _x;
        (*ent)->y = _y;
    }
    
    int main(void)
    {
        entity_t* ent;
        if(0>entity_init(&ent, 10, 24))
            return 1;
        printf("Entity: x%d y%d", ent->x, ent->y);
        return 0;
    }
    

    A more usual pattern for this would be:

    entity_t *entity_new(int _x, int _y)
    {
        entity_t *ent = malloc(sizeof(*ent));
        if (NULL==ent) 
            return NULL;
        ent->x = _x;
        ent->y = _y;
        return ent;
    }
    
    int main(void)
    {
        entity_t* ent;
        if(NULL==(ent=entity_new(10,24)))
            return 1;
        printf("Entity: x%d y%d", ent->x, ent->y);
        return 0;
    }