Confused about accessing struct members via a pointer

Question

I'm new to C, and am confused by results I'm getting when referencing a member of a struct via a pointer. See the following code for an example. What's happening when I reference tst->number the first time? What fundamental thing am I missing here?

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

    typedef struct {
       int number;
    } Test;

    Test* Test_New(Test t,int number) {
        t.number = number;
        return &t;
    }




    int main(int argc, char** argv) {

    Test test;
    Test *tst = Test_New(test,10);
    printf("Test.number = %d\n",tst->number);
    printf("Test.number = %d\n",tst->number);
    printf("Test.number = %d\n",tst->number);
}

//Output --------------------------

Test.number = 10
Test.number = 4206602
Test.number = 4206602

//Output ---------------------------

Answer 1

You are returning the address of t as declared in the method Test_New, not the address of test that you passed into the method. That is, test is being passed by value and you should instead pass a pointer to it.

So, here is what happens when you call Test_New. A new Test struct named t is created and t.number is set to be equal to the value of test.number (which you had not initialized). Then you set t.number equal to the parameter number that you passed to the method, and then you return the address of t. But t is a local variable and goes out of scope as soon as the method ends. Thus, you are returning a pointer to data that no longer exists and that is why you are ending up with garbage.

Change the declaration of Test_New to

Test* Test_New(Test* t,int number) {
    t->number = number;
    return t;
}

and call it via

Test *tst = Test_New(&test,10);

and all will go as you are expecting.

Answer 2

When you pass test into your Test_New function, you are passing it by value and so a local copy is made on the stack for the function scope of your Test_New function. Since you return the address of the variable, once the function returns the stack is useless but you've returned a pointer to a struct on the old stack! So you can see that your first call returns the correct value since nothing has overwritten your stack value but the subsequent calls (which all use the stack) overwrite your value and give you erroneous results.

To do this correctly rewrite your Test_New function to take a pointer and pass the pointer to the struct into the function.

Test* Test_New(Test * t,int number) {
    t->number = number;
    return t;
}

int main(int argc, char ** argv)  {
   Test test;
   Test * tst = Test_New(&test,10);

   printf("Test.number = %d\n",tst->number);
   printf("Test.number = %d\n",tst->number);
   printf("Test.number = %d\n",tst->number);

}

Answer 3

The problem is that you are not passing a reference into Test_New, you are passing a value. Then, you're returning the memory location of the local variable. Consider this code which demonstrates your problem:

#include <stdio.h>

typedef struct {
} Test;

void print_pass_by_value_memory(Test t) {
  printf("%p\n", &t);
}

int main(int argc, char** argv) {
  Test test;
  printf("%p\n", &test);
  print_pass_by_value_memory(test);

  return 0;
}

The output of this program on my machine is:

0xbfffe970
0xbfffe950

Answer 4

You've passed the contents of test by value to Test_New. IOW a new copy of a Test structure has been allocated on the stack when you called Test_New. It is the address of this Test that you return from the function.

When you use tst->number the first time the value of 10 is retrieved because although that stack has be unwound no other use of that memory has been made. However as soon as that first printf has been called the stack memory is reused for whatever it needs, but tst is still pointing to that memory. Hence subsquent uses of tst->number retrieve whatever printf left there in that memory.

Use Test &t in the function signature instead.

Answer 5

Just to extend BlodBath's answer, think about what happens in memory when you do this.

As you enter your main routine, a new automatic Test struct is created -- on the stack, since it's auto. So your stack looks something like

    | return address for main |  will be used at bottom
    | argc                    |  copied onto stack from environment
    | argv address            |  copied onto stack from environment
->  | test.number             |  created by definition Test test;

with -> indicating the stack pointer to the last used element of the stack.

Now you call Test_new(), and it updates the stack like this:

    | return address for main |  will be used at bottom
    | argc                    |  copied onto stack from environment
    | argv address            |  copied onto stack from environment
    | test.number             |  created by definition Test test;
    | return addr for Test_new|  used to return at bottom
    | copy of test.number     |  copied into the stack because C ALWAYS uses call by value
->  | 10                      |  copied onto stack

When you return &t, which address are you getting? Answer: the address of the data ON THE STACK. BUT THEN you return, the stack pointer is decremented. When you call printf, those words on the stack are re-used, but your address is still poiting to them. It happens that what the number in that location in the stack, interpreted as an address, points to has the value 4206602, but that's pure chance; in fact, it was kind of bad luck, as good luck would have been something that caused a segmentation fault, letting you know something was actually broken.

Answer 6

Independent of struct, it is always incorrect to return the address of a local variable. It is usually also incorrect to put the address of a local variable into a global variable or to store it in an object allocated on the heap with malloc. Generally if you need to return a pointer to an object, you'll need either to get someone else to provide the pointer for you, or else you'll need to allocate space with malloc, which will return a pointer. In that case, part of the API for your function must specify who is responsible for calling free when the object is no longer needed.

Answer 7

You could do something like this to make it a little easier:

typedef struct test {
   int number;
} test_t;

test_t * Test_New(int num)
{
   struct test *ptr;

   ptr = (void *) malloc(sizeof(struct test));
   if (! ptr) {
     printf("Out of memory!\n");
     return (void *) NULL;
   }

   ptr->number = num;

   return ptr;
}

void cleanup(test_t *ptr)
{
    if (ptr)
     free(ptr);
}

....

int main(void)
{
    test_t *test, *test1, *test2;

    test = Test_New(10);
    test1 = Test_New(20);
    test2 = Test_new(30);

    printf(
        "Test (number) = %d\n"
        "Test1 (number) = %d\n"
        "Test2 (number) = %d\n",
        test->number, test1->number, test2->number);
    ....

    cleanup(test1);
    cleanup(test2);
    cleanup(test3);

    return 0;
}

... As you can see, its easy to allocate room for several completely different instances of test_t, for instance if you need to save the existing state of one so you can revert later .. or for whatever reason.

Unless, of course there is some reason why you must keep it local .. but I really can't think of one.

Answer 8

Test t declared in *Test_New()* is a local variable. You are trying to return the address of a local variable. As the local variable gets destroyed once the function exists, the memory will be freed meaning, the compiler is free to put some other value in the location where your local variable was kept.

In your program when you are trying to access the value the second time, the memory location might have got assigned to a different variable or process. Hence you are getting the wrong output.

A better option for you will be to pass the structure from main() by reference rather than by value.