57

Assume I have defined a class like this:

 class foo {
 private: 
    std::vector< int* > v;
 public:
    ...
    void bar1()
    {
       for (int i = 0; i < 10; i++) {
         int *a = new int;
         v.push_back( a );
       }
    };

    void bar2()
    {
       std::vector< int >::iterator it = v.begin();
       for ( ; it != v.end(); it++ )  
         std::cout << (*it);
       v.clear();
    }
 };

In short, I push back some pointers in a vector, later I clear the vector. The question is, does this code has memory leak? I mean by clearing the vector, are the pointers deleted properly?

mahmood
  • 23,197
  • 49
  • 147
  • 242
  • 2
    You actually push only a single pointer to the vector; the `for` loop in `bar1` only executes the `new int;` line since it has no curly braces, and `i+++` is a syntax error and... ah well, I guess this is meant to be pseudocode. – Frerich Raabe Oct 09 '12 at 07:53
  • You also need a vector of int pointers: `std::vector< int* > v;` instead of `std::vector< int > v;` – juanchopanza Oct 09 '12 at 08:00
  • @juanchopanza: yeah, fixed... – mahmood Oct 09 '12 at 08:02

4 Answers4

74

Yes, the code has a memory leak unless you delete the pointers. If the foo class owns the pointers, it is its responsibility to delete them. You should do this before clearing the vector, otherwise you lose the handle to the memory you need to de-allocate.

   for (auto p : v)
   {
     delete p;
   } 
   v.clear();

You could avoid the memory management issue altogether by using a std::vector of a suitable smart pointer.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
15

I think the shortest and clearest solution would be:

std::vector<Object*> container = ... ;
for (Object* obj : container)
    delete obj;
container.clear();
Tim Kuipers
  • 1,705
  • 2
  • 16
  • 26
10

Nope you only clear the vector storage. Allocated memory with 'new' is still there.

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

You can use for_each :

std::vector<int*> v;

template<typename T>
struct deleter : std::unary_function<const T*, void>
{
  void operator() (const T *ptr) const
  {
    delete ptr;
  }
};

// call deleter for each element , freeing them
std::for_each (v.begin (), v.end (), deleter<int> ());
v.clear ();
tozka
  • 3,211
  • 19
  • 23
  • I often wish something like this `deleter` was readily available; I wonder, could you implement it in terms of `std::mem_fun_ptr or `std:fun_ptr` or so? – Frerich Raabe Oct 09 '12 at 07:55