nested structures and pointers

问题:

I dork this up just about every time I jump back into a C project. I'm getting a segfault when attempting to access a structure within a structure. Let's say I have the following (simplified) structures for a game:

struct vector {
    float x;
    float y;
};

struct ship {
    struct vector *position;
};

struct game {
    struct ship *ship;
} game;

And a function to initialize the ship:

static void
create_ship(struct ship *ship)
{
    ship = malloc(sizeof(struct ship));
    ship->position = malloc(sizeof(struct vector));
    ship->position->x = 10.0;
}

Then down in main():

int main() {
    create_ship(game.ship);
    printf("%f\n", game.ship->position->x); // <-- SEGFAULT
}

回答1:

You are passing game.ship by value, so inside create_ship the variable ship is just a copy of that pointer, and it just changes the copy. When the function returns, the pointer to what you malloced is lost and the effects of the function are not visible outside it, except for the memory leak.

You need to pass a pointer to the pointer and modify game.ship through that:

static void
create_ship(struct ship **ship)
{
    *ship = malloc(sizeof(struct ship));
    (*ship)->position = malloc(sizeof(struct vector));
    (*ship)->position->x = 10.0;
}

And

create_ship(&game.ship);

Or perhaps a better way would be to do as Johnny Mopp suggests in the comments, return the pointer from the function instead of modifying one outside of it:

static struct ship* create_ship()
{
    struct ship* s = malloc(sizeof(struct ship));
    s->position = malloc(sizeof(struct vector));
    s->position->x = 10.0;

    return s;
}

And

game.ship = create_ship();

And of course, what you have malloced, don't forget to free.

回答2:

You should pass by reference:

static void create_ship(struct ship **ship);

and

create_ship(&game.ship);

otherwise the allocated structure is discarded.

回答3:

Your create_ship function isn't actually changing game.ship, since the argument is passed-by-value. In the create_ship function, only a copy is being changed. So after the call, game.ship remains uninitialized (or NULL, or whatever it's value is before the call). Consequently you also lose the reference to the newly allocated ship, and leak memory.

You'll need to pass a pointer to a pointer in order to change it:

static void create_ship(struct ship **ship)
{
    *ship = malloc(sizeof(struct ship));
    // ...

And in the call: create_ship(&game.ship);

You could also opt to pass out the new ship as the function return value:

static ship *create_ship()
{
    struct ship *ship = malloc(sizeof(struct ship));
    // ...

    return ship;
}

// ...

game.ship = create_ship();