views:

278

answers:

7

How do you fix a memory leak where you're returning from the function the leak itself?

For example, I make a char* returnMe = new char[24324]; returnMe is what ends up getting returned from the function. How do you account for this memory leak? How do you destroy it once it's been returned? I have some memory management rules in place that throw runtime errors on memory leaks to stop this, so I can't really ignore it.

Orrr am I a fool and this isn't a leak, implying that the leak is elsewhere?

+3  A: 

You can still delete[] the pointer returnMe even after it is returned. It is not a memory leak unless you forget to delete returnMe.

Charles Salvia
Umm...that's delete [] returnMe. delete returnMe will usually work, but isn't actually correct.
Michael Kohne
Right - thanks. Changed it to delete[]
Charles Salvia
+2  A: 

If your function is made to return an allocated buffer, there's no leak. Better, there's a leak if the client code on purpose calls your routine and does not do anything with what it returns, but in that case, it's the client code that is violating your contract.

When the buffer gets returned, it's controlled by the client code. In general, it's a good idea for solid programming to let allocation/deallocation happen at the same layer. Meaning that if you have a routine that returns a buffer, you should have a corresponding routine that deallocates it, even if the client code could call a delete[].

For example: suppose you have this code

char *mylib_gimmeh_buffer() {
    char* returnMe = new char[24324];
    return returnMe;
}

For symmetry you should have also

char *mylib_free_buffer(buffer) {
    delete[] buffer;
}

Now, your client code should behave like this

buf = mylib_gimmeh_buffer();
/* do something with buf */
mylib_free_buffer(buf);

if your client does something like this

mylib_gimmeh_buffer(); // just for fun

or like this

buf = mylib_gimmeh_buffer();
/* do something with buf */
/* never call mylib_free_buffer() */

then there's a memory leak

Stefano Borini
+1  A: 
char* function()
{
   char* returnMe = new char[24324];
   return returnMe;
}

int main()
{
   char* pGetReturnME = function();
   delete [] pGetReturnME;
}

You can still release the memory allocated in a function only if you have a pointer to the allocated memory. If the function returns you the pointer to the memory allocated you can release using delete or delete[].

You have to be careful in having a clear protocol of allocation and deallocation of memory to avoid multiple deletion or memory leak.

aJ
Please, no `void main()`. See http://www.research.att.com/~bs/bs_faq2.html#void-main
Sinan Ünür
Thanks. Fixed the same.
aJ
+1  A: 

The calling function is responsible for freeing the returned memory.

void function() {
    char *to_free = NULL;
    //to_free does not need to be freed yet

    to_free = function_that_uses_new_to_set_return();

    //to_free now must be freed if that function worked
    if (to_free != NULL) delete[] to_free;
}
Ryan Ahearn
You certainly don't want to call `free` on a pointer that has been allocated with `new`. You should edit this post to change `free` to `delete[]`
Charles Salvia
+12  A: 

It's not a leak if you return it (well, it's not your leak).

You need to think in terms of resource ownership. If you return an allocated buffer from your function, the caller of the function is now responsible for it. The API should make it clear that it needs to be freed when they're finished with it.

Whether they free it themselves or pass it to another of your functions to have it freed (encapsulation in case more needs to be done than just freeing the memory) is another issue for the API.

paxdiablo
+1  A: 

While it's possible to have a function that allocates memory and returns a pointer to it, it's really error-prone... every time someone calls that function, they need to know that the function will be allocating memory on their behalf, and they need to know that they are responsible for freeing that memory, and they need to know the appropriate way to free the memory when they are done with it (delete? delete[]? free()? Something else?), and they need to make sure that free happens exactly once, in all cases, and they need to make sure never to dereference the pointer again after the free occurs.

Inevitably in a non-trivial program, someone (possibly you!) will get one of those things wrong. Then someone (probably you) will get to spend several hours of quality time with a debugger (or if you're lucky, valgrind) tracking down why your program is sometimes crashing or leaking memory. It's really not much fun.

Fortunately in C++ there are better ways to do this that are much harder for the caller to screw up. For example, instead of returning a raw pointer, you can return a shared_array that will automatically delete the allocated memory when the last pointer to it goes away. That way nobody has to think about memory management, it "just works".

Once you get used to doing it that way, you'll never want to go back!

Jeremy Friesner
A: 

One solution is to return the memory in a way that's hard to leak. E.g. return it as a boost::shared_ptr with a custom deleter (since you need delete[]). Now it actually takes effort by the caller to leak.

MSalters