Hey!

I was writing my code, and just became wondering if I get a memory leak here. The program has a function which checks and creates a "Time" object, which simply includes integer values for hours and minutes. It returns the Time-object created to the main code. The thing is, I use the "new" command to make the object in the function, and what I've learned so far, I should use the "delete" command to delete the object and free the memory... or do I have to, in this kind of situation? Does the function free the memory automatically when the return statement is met? :)

In the function code:

...

Time* tempTime = 0;
    if(ok)
        tempTime = new Time(hours_int, minutes_int);
    else
        cout << "Error!" << endl;
    
    return tempTime;

And, in the main:

Time* paramTime = 0;
paramTime = createTime(argv[4]);
        if(paramTime != 0) {
            ok4 = true;
            cout << "OK4" << endl;
        }

The code works perfectly, I'm just curious to know if it leaks or not. :)

or do I have to, in this kind of situation? Does the function free the memory automatically when the return statement is met?

No delete - no deletion. It would be pretty bad if the memory was freed anyway - then you'd return a pointer to a destroyed object.

But yes, it does leak - you have no delete anywhere. You need to delete paramTime after you're done with it.

Besides, returning a pointer to an object created inside the function is very bad style due to the high risk of memory leaks if you forget to delete the returned object. There are some possible solutions to this (for all it is implied that you don't return a pointer, but an object):

1) If the object can have an invalid state, return an invalid object on failure. A time class could - the invalid state could be zero for each member. An isValid() member function can be used to check for validity.
2) throw an exception if the function fails.
3) assign the time to an object passed by reference and return whether the function was successful.
4) keep using new but return a smart pointer such as auto_ptr, unique_ptr or shared_ptr.

Edited 6 Years Ago by Aranarth: n/a

I agree with Aranath when using 'new' the object will exist beyond the return statement and needs to be deleted. It doesn't cause problems because the memory leaked is small and once the program ends the memory is freed up.

Aranath I wanted to ask you how using auto_ptr or unique_ptr could help in this situation? I understand how shared_ptr could help as it will automatically delete its contents when it is no longer required (reference counted) but not clear on how the others help. Thanks.

@AkashL: The auto_ptr will help because it will make sure the "paramTime" in this case is deleted before main() exits, but it is not ideal because, as you said, it is not reference counted, so if the pointer is copied to other functions and stuff, it will get deleted at the first scope that it leaves and its copies will be pointing to a deleted object (very bad, of course).

The unique_ptr is better for two reasons. First it is non-copyable, it's guaranteed that it will not be sent anywhere that is outside the original scope, and will be automatically deleted at the end of the scope (as for that auto_ptr). Second, a unique_ptr is packaged with a deleter (optional parameter to the constructor of unique_ptr), this guarantee that the delete operator used to free the resource is the right one (this is extremely useful for cross-module implementations, where multiple heaps are involved in the overall application).

So the conclusion is that auto_ptr is not very safe in general. shared_ptr/weak_ptr and unique_ptr are more useful for the copyable-reference-counted case and the non-copyable case, respectively.

@bleedi, if you are using linux, try valgrind to diagnose memory leaks and other memory-related issues.

So... Basically I can go with this, if I just delete the paramTime after I don't need it anymore? I'm really doing my first C++ course, I have no idea about auto_ptr or such. :P

This article has been dead for over six months. Start a new discussion instead.