Freed pointer reallocaing itself?

Question

When I dynamically allocate a structure, and then try to free it, it seems to reallocate it.

typedef struct OBJ_T
{
   int param1, param2;
} OBJ;

OJB* Construct(int par1, int par2)
{
   OBJ* x = malloc(sizeof(OBJ));
   x->param1 = par1;
   x->param2 = par2;
   return x;
}

void freeup(OBJ *x)
{
   printf("x pointer -> %d ", x);
   free(x);
   x=NULL;
   printf("\nx pointer -> %d\n", x);
}

int main()
{
   OBJ foo = *Construct(10, 20);
   freeup(&foo);
   freeup(&foo);
   return 0;
}

The result is:

x pointer -> 2752216
x pointer -> 0
x pointer -> 2752216
x pointer -> 0

So after freeing and nulling out the pointer, if I check it again, it goes back to what it was. Is that a problem? Or is it free and I don't have to worry about it anymore?

Answer 1

First, this line is allocates foo on the stack and then copies dynamically allocated struct to the place:

OBJ foo = *Construct(10, 20);

Of cause, you shouldn't free it. Correct case is:

OBJ* foo = Construct(10, 20);
. . .
freeup(foo);

Second, if you want to clear pointer in freeup, then change the type of parameter to pointer-to-pointer:

void freeup(OBJ** x)
{
   free(*x);
   *x=NULL;
}

...In this case you should change the freeup call into freeup(&foo);.

Finally, it may seem that foo is reallocated in you code, but it just can't be freed or allocated, it's a stack variable. So pointer to foo is always the same inside the function.

Answer 2

You are losing your pointer in the line

OBJ foo = *Construct(10, 20);

You take the struct OBJ that the pointer returned by Construct points to, copy its values to foo and then throw away the pointer. After that, because you have no pointer, you have no way to free the actual struct OBJ allocated by Constructor.

Your freeup calls are wrong then, because you try to free foo, rather than the original.

The correct way is to either keep working with the pointer, so

OBJ *foo = Construct(10, 20);
...
freeup(foo);

or not use pointers at all and just return an OBJ in Construct, rather than an OBJ*. That way, there is no need to worry about freeing anything.

Answer 3

 OBJ foo = *Construct(10, 20);

Due to this you looses your pointer which points to block allocated by malloc . So freeing memory not allocated by malloc or similar cause error in program.

And this I really don't understand (taking in general - should not be done with pointer also )-

freeup(&foo);              // just one call is enough 
freeup(&foo);              // Why twice ?  

And this -

  void freeup(OBJ *x)
{
    printf("x pointer -> %d ", x);
    free(x);
    x=NULL;
    printf("\nx pointer -> %d\n", x);          // should not be done in any case after assigning pointer to NULL
}

You can do this instead-

  void freeup(OBJ **x)
   {
        printf("x pointer -> %d ", *x);
        free(*x);
       *x = NULL;
  }

Call it in a same manner as you do freeup(&foo);

Answer 4

You have multiple problems in your code:

1. Losing reference to the original malloc'd pointer
2. Trying to free an object on the stack
3. Assigning 0 to the copy of the pointer (passed to freeup)

(1) The line OBJ foo = *Construct(10, 20); doen't assign the pointer to the allocated OBJ to foo, but copies the OBJ's value to the stack, to a variable named foo, this causes you to lose the reference to the allocated object (and cause a memory leak).

A better way to do it is to return the pointer itself, it's clearer, doesn't cause you to lose the reference, and more efficient:

OBJ *foo = Construct(10, 20);

Now foo is a pointer to the object, not the object itself.

(2) derived from (1), now that we use foo as a pointer to the malloc'd object, we can use free on it.

(3) Assigning to a parameter from the callee does not change the value in the caller site. This is known as pass-by-value, as only a copy of the value of the argument is passed to the callee. So let's change freeup:

void freeup(OBJ **x)
{
   printf("x pointer -> %d ", *x);
   free(*x);
   *x=NULL;
   printf("\nx pointer -> %d\n", *x);
}

This way, the value of the pointer at the caller site will be changed.

---

Note that I omitted NULL checks etc. to make the code clearer, but they should definitely be there.