CODEWITHSUNDEEP
cstructpass-by-reference
user1666419
user1666419

Reputation: 77

weird issue using struct pointer in c

Ok, so i'm totally new to structs in c, and i have a problem that seems very strange to me.
When passing a simple struct to a function using it's pointer, the struct automatically takes one of the other arguments of that function as it's new data. I have no idea why this would happen.. At this moment move_walker() should do nothing at all, right?

typedef struct {
    int x,
        y;
} walker_t;

walker_t* init_walker(int x, int y) {
    walker_t walker;
    walker.x = x;
    walker.y = y;
    walker_t *pointer = malloc(sizeof(walker));
    pointer = &walker;
    return pointer;
}

int move_walker(walker_t * walker, int direction) {
    return 0;
}

walker_t* walker;
walker = init_walker(8,2);

printf("%d %d\n", walker->x, walker->y); //will print '8 2'
move_walker(walker, 3);
printf("%d %d\n", walker->x, walker->y); //will print '0 3'

(I'm pretty sure that it doesnt matter, but this code is actually spreaded over multiple files.)

Upvotes: 2

Views: 165

Answers (6)

tom
tom

Reputation: 19153

Your init_walker is wrong, because it returns a pointer to a stack-local variable, walker. That variable's memory gets reclaimed once init_walker exits. Your first printf still works, sort of by accident, because your walker variable's value is still untouched on the stack. As soon as you make any function call after that, however, the stack frame of your original init_walker call gets overwritten, and the walker pointer is now pointing to some random garbage.

When you malloc inside init_walker, you're already allocating memory on the heap (which, unlike the stack, lives beyond the lifetime of a stack frame) for your walker_t. So, you should do this instead:

walker_t* init_walker(int x, int y) {
    walker_t *pointer = malloc(sizeof(walker_t));
    pointer->x = x;
    pointer->y = y;
    return pointer;
}

Upvotes: 3

Lundin
Lundin

Reputation: 213842

...or, after a sanity check of the code, you could as well write:

typedef struct 
{
    int x,
    int y;
} walker_t;

void init_walker(walker_t* obj, int x, int y) 
{
   obj->x = x;
   obj->y = y;
}

walker_t walker;
init_walker(&walker, 8,2);

Upvotes: 0

bash.d
bash.d

Reputation: 13207

You are creating your struct-object on the stack. You'll need to allocate it using

walker_t* init_walker(int x, int y) {
walker_t* walker = malloc(sizeof(walker_t));
...
return walker;
}

With

walker_t *pointer = malloc(sizeof(walker));
pointer = &walker;

you are creating a memory-leak! You assign new memory to *pointer and lose the pointer when you assign &walker to pointer.

Upvotes: 2

Anonymouse
Anonymouse

Reputation: 945

your code is to put it polity 'very strange'.

This will work better...

walker_t *init_walker (int x, int y)
{
    walker_t *p_walker = (walker_t *)malloc (sizeof(walker));

    if (p_walker != NULL)
    {
        p_walker->x = x;
        p_walker->y = y;
    }
    return (p_walker);
}

Then call free (walker) when you've finished with them

Upvotes: 0

qPCR4vir
qPCR4vir

Reputation: 3571

 walker_t* init_walker(int x, int y) {
    walker_t walker;
    walker.x = x;
    walker.y = y;
    walker_t *pointer = malloc(sizeof(walker));
    *pointer = walker;  /* here was the error. Copy the value not the adress */
    return pointer;
}

But it can be simpler:

walker_t* init_walker(int x, int y) {

    walker_t *pointer = malloc(sizeof(*pointer));
    pointer->x = x;
    pointer->y = y;       
    return pointer;
}

Upvotes: 0

Medo42
Medo42

Reputation: 3821

The problem is that your walker pointer goes to invalid stack memory, because init_walker has a bug: You create a walker_t struct on the stack, then reserve memory with malloc and assign the address of that memory to pointer. So far so good.

However, the line pointer = &walker does not copy the struct from the stack to the new memory, instead it makes pointer point to your struct on the stack! &walker is the address of walker, and you assign that to your pointer. What you probably want to do is to copy the struct. To do that, you have to dereference your pointer:

*pointer = walker

That should make your program work as intended. You can also skip the struct on the stack completely:

walker_t* init_walker(int x, int y) {
    walker_t *walker = malloc(sizeof(walker_t));
    walker->x = x;
    walker->y = y;
    return walker;
}

Upvotes: 3

Related Questions