13

Consider the c code:

void mycode() {
  MyType* p = malloc(sizeof(MyType));
  /* set the values for p and do some stuff with it */
  cleanup(p);
}


void cleanup(MyType* pointer) {
  free(pointer);
  pointer = NULL;
}

Am I wrong in thinking that after cleanup(p); is called, the contents of p should now be NULL? Will cleanup(MyType* pointer) properly free the memory allocation?

I am coding my college assignment and finding that the debugger is still showing the pointer to have a memory address instead of 0x0 (or NULL) as I expect.

I am finding the memory management in C to be very complicated (I hope that's not just me). can any shed some light onto what's happening?

Ash
  • 24,276
  • 34
  • 107
  • 152

6 Answers6

22

Yes that will free the memory correctly.

pointer inside the cleanup function is a local variable; a copy of the value passed in stored locally for just that function.

This might add to your confusion, but you can adjust the value of the variable p (which is local to the mycode method) from inside the cleanup method like so:

void cleanup(MyType** pointer) {
  free(*pointer);
  *pointer = NULL;
}

In this case, pointer stores the address of the pointer. By dereferencing that, you can change the value stored at that address. And you would call the cleanup method like so:

cleanup(&p);

(That is, you want to pass the address of the pointer, not a copy of its value.)

I will note that it is usually good practice to deal with allocation and deallocation on the same logical 'level' of the software - i.e. don't make it the callers responsibility to allocate memory and then free it inside functions. Keep it consistent and on the same level.

sje397
  • 41,293
  • 8
  • 87
  • 103
13

cleanup will properly free p, but it won't change its value. C is a pass-by-value language, so you can't change the caller's variable from the called function. If you want to set p from cleanup, you'll need to do something like:

void cleanup(MyType **pointer) {
  free(*pointer);
  *pointer = NULL;
}

And call it like:

cleanup(&p);

Your code is a little bit un-idiomatic, can you explain a bit better why you want to write this cleanup function?

Carl Norum
  • 219,201
  • 40
  • 422
  • 469
6

Yes

Yes

Yes: There is a block of memory magically produced by malloc(3). You have assigned the address of this memory, but not the memory itself in any meaningful way, to the pointer p which is an auto variable in mycode().

Then, you pass p to cleanup(), by value, which will copy the pointer and, using the copy local to cleanup(), free the block. cleanup() then sets it's own instance of the pointer to NULL, but this is useless. Once the function is complete the parameter pointer ceases to exist.

Back in mycode(), you still have pointer p holding an address, but the block is now on the free list and not terribly useful for storage until allocated again.

You may notice that you can even still store to and read back from *p, but various amounts of downstream lossage will occur, as this block of memory now belongs to the library and you may corrupt its data structures or the data of a future owner of a malloc() block.

Carefully reading about C can give you an abstract idea of variable lifetime, but it's far easier to visualize the near-universal (for compiled languages, anyway) implementation of parameter passing and local variable allocation as stack operations. It helps to take an assembly course before the C course.

DigitalRoss
  • 143,651
  • 25
  • 248
  • 329
  • Hehehe... no, I was serious. Actually I started a long reply but then I decided to drop it because it was more a rant about how teaching programming has moved from bottom-up (the best way IMO) to top-down (that doesn't work well, basically because there is no top) to top-up (i.e. starting from ugly things like Java and going nowhere). I really believe that pointers are dead simple, but only if you have a firm grasp of how a computer works (a simple assembly is IMO a good starting point). Without that base programming becomes just a huge pile of magical words with strange properties. – 6502 May 19 '11 at 09:04
  • @6502: I totally agree - the 'user's guide' for the C64 was awesome. – sje397 May 19 '11 at 12:43
  • @6502, sure, good points. But what I "got" was your user name. Good choice. – DigitalRoss May 19 '11 at 16:30
4

This won't work as the pointer in cleanup() is local, and thus assigning it NULL is not seen by the calling function. There are two common ways of solving this.

  1. Instead of sending cleanup the pointer, send it a pointer to the pointer. Thus change cleanup() as follows:
void cleanup(MyType** pointer)
{
  free(*pointer);
  *pointer = NULL;
}

and then just call cleanup(&p).

  1. A second option which is quite common is to use a #define macro that frees the memory and cleans the pointer.

If you are using C++ then there is a third way by defining cleanup() as:

void cleanup(MyType& *pointer) { // your old code stays the same }

Ozair Kafray
  • 13,351
  • 8
  • 59
  • 84
Dov Grobgeld
  • 4,783
  • 1
  • 25
  • 36
1

There are two questions are here:

Am I wrong in thinking that after cleanup(p); is called, the contents of p should now be NULL?

Yes, this is wrong. After calling free the memory pointed by the pointer is deallocated. That doesn't mean that the content pointed by the pointer is set to NULL. Also, if you are expecting the pointer p to become NULL in mycode it doesn't happen because you are passing copy of p to cleanup. If you want p to be NULL in mycode, then you need a pointer to pointer in cleanup, i.e. the cleanup signature would be cleanup(MyType**).

Second question:

Will cleanup(MyType* pointer) properly free the memory allocation?

Yes, since you are doing free on a pointer returned by malloc the memory will be freed.

Naveen
  • 74,600
  • 47
  • 176
  • 233
1

It's not just you.

cleanup() will properly clean up your allocation, but will not set the pointer to NULL (which should IMHO be regarded as separate from cleanup.) The data the pointer points to is passed to cleanup() by pointer, and is free()ed properly, but the pointer itself is passed by value, so when you set it to NULL you're only affecting the cleanup() function's local copy of the pointer, not the original pointer.

There are three ways around this:

  1. Use a pointer to a pointer.

    void cleanup(struct MyType **p) { free(*p); *p = NULL; }
    
  2. Use a macro.

    #define cleanup(p) do { free(p); p = NULL; } while(0)
    

    or (probably better):

    void cleanup_func(struct MyType *p) { /* more complicated cleanup */ }
    #define cleanup(p) do { cleanup_func(p); p = NULL; } while(0)
    
  3. Leave the responsibility of setting pointers to NULL to the caller. This can avoid unnecessary assignments and code clutter or breakage.

Chris Lutz
  • 73,191
  • 16
  • 130
  • 183