nested structures and pointers

Question

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
}

as a side note you could use things like typedef struct {float x; float y;} vector; so you don't have to say struct each time you use the type. — none
– none, Commented Oct 2, 2012 at 17:10

Seth Carnegie · Accepted Answer · 2012-10-02 17:13:53Z

10

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.

edited Oct 2, 2012 at 17:13

answered Oct 2, 2012 at 17:07

Seth Carnegie

75.3k22 gold badges185 silver badges252 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

001 Over a year ago

Or ship * create_ship() { ... }

Michael Krelin - hacker Over a year ago

true, there's more than one road to Rome.

Seth Carnegie Over a year ago

@JohnnyMopp thanks, I wanted the OP to know why it wasn't working, but I added a bit about what you were suggesting.

Michael Krelin - hacker · Accepted Answer · 2012-10-02 17:07:52Z

2

You should pass by reference:

static void create_ship(struct ship **ship);

and

create_ship(&game.ship);

otherwise the allocated structure is discarded.

answered Oct 2, 2012 at 17:07

Michael Krelin - hacker

144k25 gold badges200 silver badges177 bronze badges

Comments

pb2q · Accepted Answer · 2012-10-02 20:55:12Z

1

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();

edited Oct 2, 2012 at 20:55

answered Oct 2, 2012 at 17:07

pb2q

59.9k19 gold badges150 silver badges152 bronze badges

Collectives™ on Stack Overflow

nested structures and pointers

3 Answers 3

3 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related