STL question: Is it necessary to delete an object from std::list

I have a question regarding to STL std::list objects.
Let we have a definition of own structure:

Code:

struct MYSTRUCT { std::string strFile; DWORD dwSize; BOOL bFlag; };

Then we create some global std:list object:

std::list<MYSTRUCT*> g_MyStructList;

Also we have a function to add objects into that list:

Code:

void AddObject(LPSTR lpstrFile, DWORD dwSize, BOOL bFlag) { MYSTRUCT* pMySrtuct = new MYSTRUCT; pMySrtuct->strFile = lpstrFile; pMySrtuct->dwSize = dwSize; pMySrtuct->bFlag = bFlag; g_MyStructList.push_front(pMySrtuct); }

My question is simple and regards to removing the objects from the list:

Code:

void RemoveObjects() { std::list<MYSTRUCT*>::iterator itr = g_MyStructList.begin(); while(itr != g_MyStructList.end()) { delete *itr; // is it necessary to delete the iterator or may be g_MyStructList.remove will do that? g_MyStructList.remove(*itr); itr = g_MyStructList.begin(); itr++; } }

The question is:
Does g_MyStructList.remove will only remove the object from the list or it also will free the memory dinamically allocated for MYSTRUCT object in AddObject function?
Also if the answer is NO (it does not delete the object, it just removes the object from the list and we must delete the iterator ourself), what should be the first: removing from the list or delete?

Code:

delete *itr; g_MyStructList.remove(*itr); or g_MyStructList.remove(*itr); delete *itr;

Thank you for the answers.

The list is a simple container and has no responsability in freing the objects. So the answer is YES, you must delete the objects. ( disable the deletion of the objects and check the memory leaks to see it for yourself).

Regarding the second question, you should first delete it and then remove it from your list. After the remove is invoked, itr will not point to a valid list item any longer.

Code:

delete *itr; g_MyStructList.remove(*itr); or T* p = *itr; g_MyStructList.remove(*itr); delete *p;

Re: STL question: Is it necessary to delete an object from std::list

Quote:

Originally posted by Miran
Then we create some global std:list object:

std::list<MYSTRUCT*> g_MyStructList;
...
Also we have a function to add objects into that list:

Your code does not store objects in the list -- it stores pointers in the list, not objects.

This code below stores objects. There is also no need for delete if you stored objects. So to answer your question, you do not "delete" objects from the list, unless the "object" is a pointer to dynamically allocated memory. The simple rule is this -- if you call "new", then you are responsible for calling "delete". If you didn't call "new", then you don't call "delete".

Code:

#include <list> std::list<MYSTRUCT> g_MyStructList; //... int main() { MYSTRUCT m; g_MyStructList.push_back( m ); // No need for delete }

Also, there is no way for the container to call delete if you did store pointers. The container has no knowledge that you placed dynamically allocated pointers in the container. You could have easily have done this:

Code:

#include <list> std::list<MYSTRUCT *> g_MyStructList; //... int main() { MYSTRUCT m; g_MyStructList.push_back(&m ); //... }

If the container were to call "delete", it would be incorrect, since the pointer is an address of a non-dynamically allocated MYSTRUCT.

Regards,

Paul McKenzie

There are a couple of problems with the code:

Code:

void RemoveObjects() { std::list<MYSTRUCT*>::iterator itr = g_MyStructList.begin(); while(itr != g_MyStructList.end()) { delete *itr; // is it necessary to delete the iterator or may be g_MyStructList.remove will do that? g_MyStructList.remove(*itr); itr = g_MyStructList.begin(); itr++; } }

1) at best, it will miss deleting an object. At worst ... take
the case where there is only 1 element. It will enter the loop,
remove the element and then :

Code:

itr = g_MyStructList.begin(); // line 1 itr++; // line 2

line 1 sets itr back to the start of the list. Since there was only
one element, itr also now equals g_MyStructList.end(). Then you
do a itr++; This is undefined.

2) There is also a performance problem. The std::list::remove()
member function removes ALL ocurrences of the value passed as
the argument. To do this it must search the ENTIRE list. So if
there are a million elements in the list, the first remove must
search through all one million elements.

Here is a method you can use to remove all elements ...

Code:

#include <algorithm> // for std::for_each() struct DeletePointer { void operator () (MYSTRUCT *pM) { delete pM; pM = 0; // optional } }; void RemoveAllObjects() { std::for_each(g_MyStructList.begin(),g_MyStructList.end(),DeletePointer()); g_MyStructList.clear(); }

If you want to keep basically the same method as in your code,
use erase() instead of remove() as follows:

Code:

void RemoveObjects() { std::list<MYSTRUCT*>::iterator itr = g_MyStructList.begin(); while(itr != g_MyStructList.end()) { delete *itr; itr = g_MyStructList.erase(itr); } }

Thanks all for the answers.
I'm sorry but I still have a question.
Let we have following situation.
As wrote Paul McKenzie we add objects to the list, not a pointers:

Code:

#include <list> std::list<MYSTRUCT> g_MyStructList; //... int main() { MYSTRUCT m; g_MyStructList.push_front( m ); // No need for delete // ... }

Since the object m was defined in main function it becomes not valid out of the scope of main. Let MyFunction is called from another thread. What will be if I want to get the object from global g_MyStructList list?
The synchronization of access the same global list from different threads is out of the scope of my question. I just want to know: Is *itr a valid MYSTRUCT object or it does not?

Code:

void MyFunction() // called from different thread { std::list<MYSTRUCT>::iterator itr = g_MyStructList.begin(); // Is *itr a valid MYSTRUCT object? // ..... }

Thank you.

it is!

of course it is:

g_MyStructList.push_front( m );

makes a copy of the memory block;
its the same as:

struct {
int i;
int j;
} TMyStruct;

void main(char**argv, int argi) {

TMyStruct s1, s2;

s1.i = 5;
s1.j = 6;

s2 = s1;

// s2 is 5, 6

}

it is because the push_back ( type T )
is a 'call by value', that means the data is copied over the stack!

hope this helps

Thank you r.muehlbauer.

Quote:

Originally posted by Miran
Thanks all for the answers.
Since the object m was defined in main function it becomes not valid out of the scope of main.
...
The synchronization of access the same global list from different threads is out of the scope of my question. I just want to know: Is *itr a valid MYSTRUCT object or it does not?

Yes it is valid.

You're right, m is out of scope, but STL containers store copies of objects. This is why the objects that are placed in containers must have proper copy and assignment semantics. Just as if you declared a local "int" variable and had a vector<int>, you wouldn't worry about the int going out of scope, right? Same thing with MYSTRUCT and with any other classs that has proper copy semantics. This is the beauty of C++ -- you can have a type that acts just like a built-in type.

Just to add, if your object T does not pass this test without error, it isn't eligible for safe storage in a vector:

Code:

int main() { T a; // Maybe set some values for a using a's member functions. //.... T b = a; // Copy constructor T c; c = a; // assignment operator } // destruction of a, b, c

The code is simple, but it is a good test. Basically, the type T is being constructed, copy constructed, assigned, and destroyed. If the code above doesn't compile using your type T, then T isn't eligible to be placed in a vector.

If the code above compiles, but at run-time causes memory exceptions, faults, etc. at either copy constructor, assignment, or destruction, then it isn't safely eligible to be placed in a vector since these are the operations that the vector will be doing "behind the scenes".

Usually, the types that pass the compile test, but fail the runtime tests are classes and structs that contain pointers to dynamically allocated memory, and the coder did not write a proper copy constructor, assignment operator, and destructor (the rule of three).

Note that if T is an int, there is no problem. If T is a MYSTRUCT there also is no problem, since each member is safely copyable (std::string, a BOOL, and a DWORD). If you were to add an int* member, and the int* member pointed to dynamically allocated memory, then you would have a problem without coding a copy constructor, assignment op, and destructor to handle the copying of such objects. This is why I always state safely copyable as opposed to just copyable.

Regards,

Paul McKenzie

I appreciate your help Paul McKenzie.
Thank you.