STL map and dynamic memory

This is a discussion on STL map and dynamic memory within the C++ Programming forums, part of the General Programming Boards category; sup guys Just joined the forums. Im having some STL problems with deleting dynamic memory. The problem happens when I ...

  1. #1
    Registered User
    Join Date
    Feb 2005
    Posts
    4

    Post STL map and dynamic memory

    sup guys
    Just joined the forums. Im having some STL problems with deleting dynamic memory.
    The problem happens when I get to the last element left in the map. My guess is im not revalidating the iterator right.
    Any Ideas?

    Code:
    typedef map <int, AnimFile*> ANIM_FILE_MAP;    
    
    ANIM_FILE_MAP::iterator fileItr;
    for (fileItr = currNode->nodeAnimMap.begin();
     fileItr != currNode->nodeAnimMap.end();)
    {	
        AnimFile* tempInfo = fileItr->second;
        delete [] tempInfo->anims;
        delete [] tempInfo->localOffsets;
        delete tempInfo;
    
        fileItr = currNode->nodeAnimMap.erase(fileItr);
    }

  2. #2
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,677
    That shouldn't even compile. map<> does not have a member function erase() that takes an iterator and returns an iterator.

    You don't need to call erase(). Call the clear() method on the container after iterating over all the elements.

    gg

  3. #3
    Registered User
    Join Date
    Feb 2005
    Posts
    4
    Wow!! your right according to SGI, yet visual studios library says yes. Thats messed up. In either case I got rid of the erase and used clear() after the loop yet it still hangs up when I reach the last element left.

  4. #4
    Registered User
    Join Date
    May 2003
    Posts
    82
    I spent a few minutes trying to grok what's going on there, and I'm thinking that this might be dangerously confusing.

    To start you:
    - dynamically allocate a new object of type AnimFile
    - store the pointer to the object in a map
    - Then you dynamically allocate memory for its members, anims and localOffsets

    So after you do what needs to be done, you have to remember to:

    - Loop through the map
    - deallocate all the member's memory
    - deallocate the AnimFile object
    - tip-toe around a map full of dangling pointers

    The class encapsulation lets you hide this detail. For example, you could set anims and localOffsets using functions, taking care of the allocation inside the class. You could then leave the deallocation to the destructor (guaranteeing the memory is always freed). If you needed to clear the data at some arbitrary point, you could implement a clear() function (which would incidentally make up the meat of your destructor as well).

    Another idea, which could limit functionality (polymorphic behavior), is to create a map full of AnimFile objects, instead of pointers to AnimFile objects. As soon as the map goes out of scope (or is cleared by you), all the objects would be automatically destroyed. By using the iterators, you get pointer like behavior. And in a map, adding or removing elements does not invalidate any other pointers.

    Maybe some of that will be helpful. Or maybe it doesn't at all fit with what you have.

    I can't see your problem, but if you post more code (enough I could compile) I could be of more direct help. If not perhaps someone else will spot the problem.

  5. #5
    Registered User
    Join Date
    May 2003
    Posts
    82
    One other thing, your idea of walking through the nodes in the body of the for loop instead of it's definitions seems kinda backwards. Was there a particular reason behind it?

  6. #6
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    Personally I'd say use smart pointers (NOT std::auto_ptr, but boost::shared_ptr) instead of raw pointers inside the map and leave the memory handling to them.

    Oh, I just realized that AH_Tze is right, there's a definite lack of encapsulation there. AnimFile should indeed delete its contained pointers by itself in the destructor.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  7. #7
    Registered User
    Join Date
    Mar 2003
    Posts
    580
    boost smart pointers are good, but deleting memory is possible on std::vectors as well, and this is how I do it:

    Code:
    #include    <algorithm>    //Otherwise anything that uses these has to manually include this
    template<class T>
    struct VectorDeleter
    {        
        void operator()(T * & obj)     //reference to a pointer, not a pointer to a reference 
        {            
            if(obj)        
            {            
                delete obj;                
                obj = NULL;    //I don't know if this actually does anything...hmm    
            }    
        }
    };
    
    //Deleter defined in GAMEEXPORT.H right now
    #define    DELETE_MEMORY_AND_ERASE(a,t)    std::for_each(a.begin(),a.end(),VectorDeleter<t>());    a.clear();
    #define    DELETE_INDEXED_MEMORY_AND_ERASE(vec,type,ptr)    std::for_each(ptr,ptr+1,VectorDeleter<type>());    vec.clear();
    The algorithm header file is for std::for_each

    the first macro deletes all of the dynamic memory on the vecotr, the second only deletes the memory at one index into the vector (so you can delete the i'th value in the array).

    It's not immediately clear to understand, and I had some help generating this code so it's not like I 100% wrote this on my own.
    See you in 13

  8. #8
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    You have macros and no exception safety. I prefer smart pointers...

    Also, the if(obj) is useless.
    And the second macro is hard to use. Given an index, I have to do:
    Code:
    DELETE_INDEXED_MEMORY_AND_ERASE(myvec, int*, myvec.begin()+index);
    When I could do
    Code:
    delete myvec[index];
    myvec[index] = 0;
    which is shorter AND more readable.
    Last edited by CornedBee; 02-15-2005 at 08:30 AM.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  9. #9
    Registered User
    Join Date
    Feb 2005
    Posts
    4
    Thanks for the replies, especially Ah_Tze. AnimFile is acually a struct which is why I need to loop through the map and delete everything before calling clear. But you make a good point about encapsulating it.

  10. #10
    Carnivore ('-'v) Hunter2's Avatar
    Join Date
    May 2002
    Posts
    2,879
    >>yet visual studios library says yes.
    Just a quick comment: I'm not sure what version of visual studio you have, but on mine it adds a disclaimer:
    "Note that this return value is nonstandard behaviour."
    Just Google It. √

    (\ /)
    ( . .)
    c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.

  11. #11
    Registered User
    Join Date
    Mar 2003
    Posts
    580
    How is the if(obj) useless? You may be right, but I dont' exactly understand why (maybe the syntax should be different, i.e the * and & should be switched, but I haven't had problems with it). Isn't that the equivalent of:

    Code:
    if(MyVec[0])
    {
    delete MyVec[0];
    MyVec[0] = 0;
    }
    because
    Code:
    std::vector::operator[]
    returns a reference, and doing

    if(MyVec[0])

    is checking whether the pointer is already nulled through a reference.

    One other thing:
    When I want to deallocate memory from a std::vector I typically also want to get rid of the raw pointer. So, doing what you do, you have to add a line of code something like:

    Code:
    std::vector<type>::iterator ptr = MyVec.begin();
    delete MyVec[0]; //std::vector::operator [] returns a reference
    
    MyVec[0] = 0;
    
    MyVec.erase(ptr);
    erase in itself doesn't delete memory, just raw pointers. So, the code I showed doesn't delete memory that's already nulled, and it also can get rid of pointers all in one step.
    Last edited by Darkness; 02-15-2005 at 11:30 AM.

  12. #12
    Carnivore ('-'v) Hunter2's Avatar
    Join Date
    May 2002
    Posts
    2,879
    >>is checking whether the pointer is already nulled through a reference.
    Indeed, and that's why it's bad The C++ language guarantees that deleting a NULL pointer will do nothing, so you're just wasting CPU cycles by adding an unnecessary branching statement.

    >>When I want to deallocate memory from a std::vector
    Just like to point out, the OP is dealing with a std::map..


    Ted, have you updated your code (and are you still having the problem)? It would be helpful to see what you're working with at the moment
    Last edited by Hunter2; 02-15-2005 at 11:36 AM.
    Just Google It. √

    (\ /)
    ( . .)
    c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.

  13. #13
    Registered User
    Join Date
    Mar 2003
    Posts
    580
    Isn't the process for handling dynamic memory pretty much the same for each stl type (it seems it is which is why I thought my code might help).

    I also just took out that if statement

  14. #14
    Carnivore ('-'v) Hunter2's Avatar
    Join Date
    May 2002
    Posts
    2,879
    Well, you do have to delete the dynamic memory etc. as you would in a vector. On the other hand, as Codeplug mentioned you can't really erase elements in a std::map like you can in a std::vector.. so you'd probably just iterate through the map deleting all dynamic memory, and then when you're done you'd call clear() to remove all elements of the map. And that's where it's a little bit different Although, apparently, that isn't where the problem lies in Ted's code.
    Just Google It. √

    (\ /)
    ( . .)
    c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.

  15. #15
    Registered User
    Join Date
    May 2003
    Posts
    82
    Quote Originally Posted by Hunter2
    ...On the other hand, as Codeplug mentioned you can't really erase elements in a std::map like you can in a std::vector...
    ?

    from SGI ( http://www.sgi.com/tech/stl/Map.html )
    Code:
    void erase(iterator pos)
    size_type erase(const key_type& k) 
    void erase(iterator first, iterator last)
    Codeplug said there wasn't an erase function that returned an iterator (which makes sense).

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. memory leaks
    By TehOne in forum C Programming
    Replies: 4
    Last Post: 10-10-2008, 09:33 PM
  2. Replies: 2
    Last Post: 09-28-2006, 01:06 PM
  3. How do you search a list container....
    By chadsxe in forum C++ Programming
    Replies: 22
    Last Post: 07-16-2005, 12:39 PM
  4. Using the map STL
    By Sethiorth in forum C++ Programming
    Replies: 2
    Last Post: 10-22-2003, 03:07 PM
  5. Dynamic memory
    By frenchfry164 in forum C++ Programming
    Replies: 10
    Last Post: 07-10-2002, 09:31 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21