CODEWITHSUNDEEP
c++c++11memory
Aso Strife
Aso Strife

Reputation: 1129

Memory deallocation inside a member function

I was thinking about a this situation not for a real implementation but to understand better how pointers works.

class foo(){
    foo();
    ~foo();
    void doComplexThings(const std::vector<int*>& v){
        int* copy;
        for(int i = 0; i < v.size(); i++){
           copy = v[i];
           // do some stuffs
        }
    }    
}

main(){
    std::vector<int*> myVector; // suppose we have 100 elements

    doComplexThings(myVector);

    for(int i = 0; i < myVector.size(); i++){
         delete myVector[i];
    }
    myVector.clear();    
}

Ok, I know that have no sense to copy v[i] inside an other pointer, but I was thinking: copy do a memory leak? After the execution of doComplexThings(), copy will continue to exist and will occupy space in the heap?

After deleting all elements it will continue to exist and point to a deallocated memory?

So logically if I do this things with complex objects I'll keep occupy the memory with unreference object? Or copy is saved in the stack because I don't use new? And at the end of doComplexThings it will be deleted?

I'm a bit confused, thanks!

Upvotes: 1

Views: 645

Answers (3)

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726599

No additional memory is allocated on the heap when you do this:

copy = v[i];

variable copy points to the same address as v[i], but no additional array is allocated, so there would be no memory leak.

A better way of dealing with the situation is to avoid raw pointers in favor of C++ smart pointers or containers:

std::vector<std::vector<int>> myVector;

Now you can remove the deletion loop, which is an incorrect way of doing it for arrays allocated with new int[length] - it should use delete[] instead:

delete[] myVector[i];

Upvotes: 1

patatahooligan
patatahooligan

Reputation: 3321

There is some confusion on the topic of pointers in the C++ community. While it is true that smart pointers have been added to the library to alleviate problems with dynamic memory allocation, raw pointers are not obsolete. In fact, whenever you want to inspect another object without owning it, you should use a reference or raw pointer, depending on which suits your needs. If the concept of ownership is unclear to you, think of an object as being owned by another object if the latter is responsible for cleaning up afterwards (deleting the former).

For example most uses of new and delete should be replaces with the following (omitting std for brevity):

{
    auto ptr_to_T = make_unique<T>(//constructor params);
    do_stuff_with_smart_ptr(ptr_to_T);
    do_stuff_with_T(*ptr_to_T);
    do_stuff_with_raw_ptr(ptr_to_T.get());
} // automatic release of memory allocated with make_unique()

Notice how a function that takes a T* doesn't need a smart pointer if it doesn't keep a copy of the T* it is given, because it doesn't affect the lifetime of the object. The object is guaranteed to be alive past the return point of do_stuff_with_T() and its function signature signals that it doesn't own the object by taking a raw pointer.

On the other hand, if you need to pass the pointer to an object that is allowed to keep the pointer and reference it later, it is unclear when the object will need to be destroyed and most importantly by whom. This is solved via a shared pointer.

ClassThatNeedsSharedOwnership shared_owner;
{
    auto ptr_to_T = make_shared<T>(//constructor params);
    shared_owner.set_T(ptr_to_T);
    // do a lot of stuff
}
// At this point ptr_to_T is destroyed, but shared_owner might keep the object alive

So how does the above factor in to your code. First of all, if the vector is supposed to own (keep alive) the ints it points to, it needs to hold unique_ptr<int> or shared_ptr<int>. If it is just pointing to ints held by something else, and they are guaranteed to be alive until after the vector is destroyed, you are fine with int*. In this case, it should be evident that a delete is never necessary, because by definition your vector and the function working on the vector are not responsible for cleaning-up!

Finally, you can make your code more readable by changing the loop to this (C++11 which you've tagged in the post):

for (auto copy : v){
    // equivalent to your i-indexed loop with copy = v[i];
    // as long as you don't need the value of i
    do_stuff_to_int_ptr(copy);
    // no delete, we don't own the pointee
}

Again this is only true if some other object holds the ints and releases them, or they are on the stack but guaranteed to be alive for the whole lifetime of vector<int*> that points to them.

Upvotes: 1

Malcolm McLean
Malcolm McLean

Reputation: 6404

Basically you're illustrating the problem with C pointers which lead to the introduction of C++ unique and shared pointers. If you pass a vector of allocated pointers to an opaque member function, you've no way of knowing whether that function hangs onto them or not, so you don't know whether to delete the pointer. In fact in your example you don't seem to, "copy" goes out of scope.

The real answer is that you should only seldom use allocated pointers in C++ at all. The stl vector will serve as a safer, easier to use version of malloc / new. Then you should pass them about as const & to prevent functions from changing them. If you do need an allocated pointer, make one unique_ptr() and then you know that the unique_ptr() is the "owner" of the memory.

Upvotes: 0

Related Questions