Problems using structs with pointers

Question

I am trying to make a ball bounce side to side in ncurses. I can get this to work fine if I print the ordinary struct and pass the pointer of said struct through functions.

When I run the following code, it prints what I believe to be the memory addresses of the structs elements and not the actual values contained at those addresses. What I don't understand is why the first piece of code doesn't work, but the second does. I'm quite sure I'm messing up with pointers but I don't know where.

The bad code,

typedef struct Ball ball;

int width=80, height=20; //screen height/width in characters

struct Ball{
    char shape;
    int x;
    int y;
    int velX; 
    int velY;
};

ball* initBall(int X, int Y, int velx, int vely, char shape){
    ball b;
    ball *p = &b;
    p->x = X;
    p->y = Y;
    p->velY = vely;
    p->velX = velx;
    p->shape = shape;

    return p;
}

void moveBall(ball *b){
    if(b->x +b->velX > width || b->x + b->velX < 0){
        b->velX *= -1;
    }

    if(b->y +b->velY > height || b->y +b->velY< 0){
        b->velY *= -1;
    }

    b->x += b->velX;
    b->y += b->velY;
}

int main(){
    ball *p = initBall(40,10,1,0, 'O');

    int counter=0;
    while(counter < 10){
        printf("%d, %d\n", p->x, p->y);

        moveBall(p);

        counter++;
    }

    return 0;
}

The good code,

typedef struct Ball ball;

int width=80, height=20; //screen height/width in characters

struct Ball{
    char shape;
    int x;
    int y;
    int velX; 
    int velY;
};

ball initBall(int X, int Y, int velx, int vely, char shape){
    ball b;
    ball *p = &b;
    b.x = X;
    b.y = Y;
    b.velY = vely;
    b.velX = velx;
    b.shape = shape;

    return b;
}

void moveBall(ball *b){
    if(b->x +b->velX > width || b->x + b->velX < 0){
        b->velX *= -1;
    }

    if(b->y +b->velY > height || b->y +b->velY< 0){
        b->velY *= -1;
    }

    b->x += b->velX;
    b->y += b->velY;
}

int main(){
    ball b = initBall(40,10,1,0, 'O');
    ball *p = &b;

    int counter=0;
    while(counter < 10){
        printf("%d, %d\n", p->x, p->y);

        moveBall(p);

        counter++;
    }

    return 0;
}

If you had compiled with warnings enabled, you would have gotten a warning like this: `"warning: returning address of local variable"`... that should ring a bell. — , Nov 01 '13 at 09:07

Filipe Gonçalves · Answer 1 · 2013-11-02T19:49:52.287

In the bad code, you are returning a pointer to a local variable inside initBall. This pointer will be invalid once you return from initBall, and the contents of the struct will most likely be overwritten with other data when you call another function.

In the good code, you're not returning a pointer to struct, you're returning the whole struct - that is, a copy of the struct, so this is not a problem. Stick to the good code if you don't mind copying the whole struct when you return (which can be inefficient); otherwise, use dynamic memory allocation.

score 1 · Accepted Answer · answered Nov 01 '13 at 09:08

In bad code of initBall()

ball* initBall(int X, int Y, int velx, int vely, char shape){
    ball b;
    ball *p = &b;
    ...
    return p;
}

you are returning address of local variable, which is not correct. The contents of that memory location will change once the function returns.

While in good code you return the structure as value, so the values are copied in main().

The fix would be:

ball* initBall(int X, int Y, int velx, int vely, char shape){
    ball b;
    ball *p = malloc(sizeo(*p));

    ...
    return p;
}

Don't forget to free() it when done.

score 0 · Answer 3 · answered Nov 01 '13 at 09:06

You have undefined behavior in initBall when you return a pointer to a local variable. Remember that the scope of the variable b end with the function, so after the function returns the pointer to it is no longer valid.

You might want to allocate the structure on the heap instead.

score 0 · Answer 4 · answered Nov 01 '13 at 09:06

You are returning the address of a local variable, which then immediately becomes invalid due to the function in which the variable existed exiting.

Don't do this, it's not valid code.

You must either make the allocation static, or use malloc() to allocate heap memory which does not go out of scope.

This:

ball* initBall(int X, int Y, int velx, int vely, char shape){
  ball b;
  ball *p = &b;

should be:

ball * initBall(int X, int Y, int velx, int vely, char shape){
  ball *b = malloc(sizeof *b);
  if(b != NULL) {
    b->x = X;
    /* and so on */
  }
  return b;
}

Problems using structs with pointers

4 Answers4