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

Question

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.

Answer 1

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;
}

Answer 2

If you must allocate within the entity_init() function, you need to either return a pointer to the allocation, or add a layer of indirection by making ent a pointer to pointer to entity_t. In the posted code, within entity_init() ent is a copy of the pointer which was passed to the function. Any changes made to this pointer, such as assigning the address of a memory allocation to the pointer, will not be visible from the calling function since this copy will cease to exist after the function returns.

Also, note that you need to check the value returned from malloc() to be sure that the allocation was successful. If successful, the function can continue with the initialization process; if not, ent can remain a null pointer, which should be checked in the calling 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 = malloc(sizeof **ent);
    if (*ent) {
        (*ent)->x = _x;
        (*ent)->y = _y;
    }
}

int main(void)
{
    entity_t *ent;
    entity_init(&ent, 10, 24);

    if (ent == NULL) {
        fprintf(stderr, "Allocation failure in entity_init()\n");
        exit(EXIT_FAILURE);
    }

    printf("Entity: x%d y%d\n", ent->x, ent->y);

    return 0;
}

Program output:

Entity: x10 y24