CODEWITHSUNDEEP
cfunctionpointersstruct
Drakalex
Drakalex

Reputation: 1538

Passing a pointer to a function doesn't work as expected

I have this tiny code :

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

typedef struct Ship Ship;

struct Ship
{
    int x, y;
};

int main()
{
    Ship* ship;

    assignShip(ship);

    printf("%d", ship->x);

    return 0;
}


void assignShip(Ship* ship)
{
    Ship anotherShip;
    anotherShip.x = 10;

    ship = &anotherShip;
}

And it doesnt' work. I create a pointer of type Ship named ship, and then I pass the pointer to my function, I set the pointer to anotherShip and then tries to see if it worked but the console goes in "isn't responding" and crash. What am I doing wrong ?

Upvotes: 1

Views: 1700

Answers (3)

Elad Hazan
Elad Hazan

Reputation: 331

Ship anotherShip; is declared on the stack , once you leave the function the pointer is undeclared which means you assigned undeclared pointer . either alloc it or make it locally static or globally and it should work

Upvotes: 0

bolov
bolov

Reputation: 75688

There are ... some issues with your code and approach:

Parameters are passed by value

In C parameters are passed by value. This means that assignShip receives a copy of the pointer you have in main. So this assignment ship = &anotherShip; has no effect on the Ship* ship; variable from main, because you modify the local copy the function has.

The fix to this is to have a pointer to the main variable. I.e.:

int main()
{
    Ship* ship;

    assignShip(&ship); // we pass a pointer to the variable `ship`

    printf("%d", ship->x);

    return 0;
}


void assignShip(Ship** ship) // receive pointer to pointer
{
    Ship anotherShip;
    anotherShip.x = 10;

    *ship = &anotherShip; // ... still wrong
}

This however brings us to the next big issue you have:

Local function variables have automatic storage duration

anotherShip is local to the function assignShip which means that as soon as the function call ends that variable is destroyed, so in main you will be left with a dangling pointer

The fix to this is to use dynamic allocation:

int main()
{
    Ship* ship;

    assignShip(&ship);

    printf("%d", ship->x);

    return 0;
}


void assignShip(Ship** ship) // receive pointer to pointer
{
    Ship* anotherShip = malloc(sizeof(Ship));
    anotherShip->x = 10;

    *ship = anotherShip;
}

Now we have an object will will survive the call to assignShip.

And this brings us to the next problem:

Object with dynamic storage duration must have the lifetime managed by the programmer

Unlike automatic storage duration where the compiler is responsible to manage the lifetime of objects, with dynamic storage duration that is the responsibility of the programmer. So every memory allocated with malloc must be released with free:

int main()
{
    Ship* ship;
    assignShip(&ship);

    printf("%d", ship->x);

    free(ship); // free the memory
    return 0;
}

And this brings us to the next issue:

Ownership of manually managed resources must be clear

Although the above version of our program is technically correct, there is a problem with they way we handle memory management.

Consider this snippet:

Ship* ship;
assignShip(&ship); // we pass a pointer to the variable `ship`

Does assignShip allocate memory for the parameter. Do we have to call free after it or does it expect to receive a pointer to a valid memory location?

To answer this we must consult the documentation of assignShip (if that exists) and/or the source code of assignShip

One approach is to make that clear from the function name. e.g. allocateAndAssignShip.

The other one is to move the allocation out of the function:

int main()
{
    Ship* ship = malloc(sizeof(Ship));
    assignShip(ship);

    printf("%d", ship->x);

    free(ship);
    return 0;
}


void assignShip(Ship* ship)
{
   ship->x = 10;
}

Upvotes: 2

abelenky
abelenky

Reputation: 64672

"What am I doing wrong ?"

A bunch of things.

First, inside function assignShip, the local variable anotherShip goes out of scope at the end of the function, leaving ship pointing to a variable that no longer exists.

Secondly, since you are passing variable ship by value, not reference, you cannot make permanent changes to it from within the function.

Instead:

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

typedef struct Ship Ship;

struct Ship
{
    int x, y;
};

int main()
{
    Ship* ship = NULL;
    assignShip(&ship);
    // Check ship here, it is now non-NULL.
    printf("%d", ship->x);
    return 0;
}

void assignShip(Ship** ship)
{
    static Ship anotherShip;
    anotherShip.x = 10;
    *ship = &anotherShip;
}

Upvotes: 1

Related Questions