Cfun
Cfun

Reputation: 9721

memory mangement of list of pointers containning vectors

I am testing this code to see how program memory is managed, thanks to windows resource monitor.

class A {
public:
        string name_;
        vector<double> H;
        vector<float> G;
        vector<string> I;
        int abc;
};
list<A *> test (90000);

void allocate_class() {
    for (list<A *>::iterator it= test.begin(); it != test.end();it++) {
         A *ptr=*it;
         ptr = new A;
    }
}

void init_fct() {
    for (list<A *>::iterator it= test.begin(); it != test.end();it++) {
        A *ptr=*it;
         /* 
        all above doesnt work program crash

        (*ptr).name_ = "hello test";
        ptr->name_ = "hello test";
        ptr->abc = 57; 
        */

    }
}

void erase_class() {
     list<A *>::iterator it= test.begin();
     while( it != test.end() )
         it = test.erase(it);
 }

void delete_ptr() {
    for (list<A *>::iterator it= test.begin(); it != test.end();it++) {
        A *ptr=*it;
        delete ptr;
    }
}

int main()
{
int t;

cout << "before allocation" << endl;
cin >> t;

allocate_class();

cout << "after allocation" << endl;
cin >> t;

init_fct();

cout << "after init" << endl;
cin >> t;

erase_class();

cout << "after erase" << endl;
cout << test.size();
cin >> t;

delete_ptr();
cout << "after delete" << endl;
cout << test.size();
cin >> t;

Questions:

  1. Is the operator delete section really needed in this case ? (not sure if it's really free space)

  2. What am I doing wrong/missing in init_fct() ?

Note: (attributes are public on purpose, cin is to pause the program and check memory state)

Upvotes: 1

Views: 54

Answers (1)

Daniel Jour
Daniel Jour

Reputation: 16156

     A *ptr=*it;
     ptr = new A;

This makes a copy of the (uninitialised) pointer stored in your list, and then assigns to that copy (and not the pointer on the list) the pointer to your newly allocated instance of A. Then the copy goes out of scope, and you have a memory leak.

And of course, since the pointers in the list haven't changed, using them later leads to undefined behaviour.

To fix that, change the pointers in the list:

     *it = new A;

Concerning your erase_class function: It's now correct, in the sense that it clears the list. But as you call it before deleting the pointers in the list, you still have a memory leak:

You first remove all pointers from your list, then try to delete the allocated memory. But since you don't have any pointers left in your list, iterating over it will just do nothing.

Looking at your goal, examining memory usage, it's probably something along this what you want:

list<A*> l;
cin >> dummy;
// The list will allocate space to hold the pointers:
l.resize (90000);
cin >> dummy;
// Allocate instances for each pointer in the list
for (auto & ptr : l) {
  ptr = new A{};
}
cin >> dummy;
// Free the allocated instances
for (auto & ptr : l) {
  delete ptr;
}
cin >> dummy;
// Erase all elements from the list.
// Note that this is not necessary, when
// the list goes out of scope at the end
// of main this is done by the list destructor.
// Moreover, there's a function to erase
// all elements from the list:
//  l.clear();
auto it = l.begin();
while (it != l.end()) {
  it = l.erase(it);
}
cin >> dummy;

Note that using smart pointers (or in this case no pointers at all) as elements of your list frees you from the burden to manually delete the allocated memory.

Upvotes: 2

Related Questions