STL question: Is it necessary to delete an object from std::list
CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 9 of 9

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

  1. #1
    Join Date
    Jan 2004
    Posts
    13

    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.

  2. #2
    Join Date
    Aug 2002
    Location
    Cluj-Napoca,Romania
    Posts
    3,496
    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;
    Har Har

  3. #3
    Join Date
    Apr 1999
    Posts
    27,418

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

    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
    Last edited by Paul McKenzie; January 28th, 2004 at 05:46 AM.

  4. #4
    Join Date
    Aug 2000
    Location
    West Virginia
    Posts
    7,560
    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);
    	}
    }
    Last edited by Philip Nicoletti; January 28th, 2004 at 07:28 AM.

  5. #5
    Join Date
    Jan 2004
    Posts
    13
    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.

  6. #6
    Join Date
    Jan 2004
    Posts
    2

    Lightbulb 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

  7. #7
    Join Date
    Jan 2004
    Posts
    13
    Thank you r.muehlbauer.

  8. #8
    Join Date
    Apr 1999
    Posts
    27,418
    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

  9. #9
    Join Date
    Jan 2004
    Posts
    13
    I appreciate your help Paul McKenzie.
    Thank you.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  


Azure Activities Information Page

Windows Mobile Development Center


Click Here to Expand Forum to Full Width

This is a CodeGuru survey question.


Featured


HTML5 Development Center