CODEWITHSUNDEEP
c++memory-leaks
Shredderroy
Shredderroy

Reputation: 2920

Deallocating memory in C++ vector<T>

The following is the best "minimum working example" I can construct for now. I would like to understand if the following code leaks memory.

// Class CTest
class CTest {
  vector<int> Elements;
  CTest (vector<int>&);
  ~CTest ();
};
CTest::CTest (vector<int>& Elements_) {
  this->Elements = Elements_;
}
CTest::~CTest () {
}

// main
int main (int argc, char *argv[]) {
  vector<CTest> V;
  for (auto i = 0; i < 10; i++) {
    vector<int> U;
    for (auto j = i; j < i + 5; j++) U.push_back (j);
    V.push_back (*(new CTest (U)));
  }
  // Do whatever
  return 0;
}

Am I correct in thinking that since there isn't a corresponding invocation of delete for each invocation of new, this programme does indeed leak memory?

Upvotes: 1

Views: 545

Answers (4)

Bo Persson
Bo Persson

Reputation: 92261

Yes, it is leaking but just because you are trying too hard.

Using this code to populate the V vector

V.push_back(CTest(U));

will do what need, without any leaks.

Generally, the standard containers will manage the memory for you (without leaks). You don't have to bother.

Upvotes: 0

Kerrek SB
Kerrek SB

Reputation: 477010

Yes, you are correct. Moreover, your code is of the type "trying very hard to get it wrong", since vector is already a dynamic container and you have no reason to perform another dynamic allocation for your element (just to have it copied).

There are many more ways to screw up. None of those are a particular design problem of C++, but there's simply a limit to what the language can stop you from doing. Some more examples:

int main(int argc, char **argv)
{
    new double;  // leak
    delete static_cast<void*>(&argc);  // doesn't belong to you
    int a = *static_cast<int const *>(0x42); // not a valid pointer

    { T x; x.~T(); }  // call destructor of non-existent object

    { T y; y.~T(); new (static_cast<void*>(&y) T(); }
                   // impossible to recover from exception in T::T()
}

Fortunately, it is almost always fairly obvious that you're doing something you're not supposed to.

Upvotes: 3

user677656
user677656

Reputation:

Yes, that is correct. You don't deallocate the assignments made with new. Also, you don't need dynamic allocation within an already dynamic container.

Upvotes: 0

Oliver Charlesworth
Oliver Charlesworth

Reputation: 272487

Yes, there is a memory leak. push_back copies its argument, so the original is lost forever (there is no longer a pointer to it).

Upvotes: 9

Related Questions