1

I had to delete some of the entries from c++ std::map, for that purpose I iterate over the std::map and used erase. But I get an error. The error is like :

0x0017f3f6 in std::_Rb_tree_rebalance_for_erase () from /usr/local/lib/libstdc++.so.6
(gdb) bt
#0  0x0017f3f6 in std::_Rb_tree_rebalance_for_erase () from /usr/local/lib/libstdc++.so.6
#1  0x00487ef4 in getLetters () from /usr/lib/libmail.so
#2  0x0048cacc in getHeader () from /usr/lib/libmail.so

#6  0x08048802 in __gxx_personality_v0 ()
#7  0x00251e9c in __libc_start_main () from /lib/libc.so.6
#8  0x080486a1 in __gxx_personality_v0 ()
(gdb)

The code was something like this:

int nlsize=LettersMap.size();

if(nlsize > MAXNL)              
{                               
        std::map <std::string,MSG_HEADER>::iterator it;
        int i=0;                
        for( i=0, it=LettersMap.begin();  i <nlsize-MAXNL  ;i++, it++)
        {                       
                LettersMap.erase(it);                 
        }                       
}   

Can someone please point out the error

Global Warrior
  • 5,050
  • 9
  • 45
  • 75
  • possible duplicate of [Problem with std::map::iterator after calling erase()](http://stackoverflow.com/questions/4636182/problem-with-stdmapiterator-after-calling-erase) – rerun Jan 24 '12 at 05:08
  • That's a somewhat unusual way to use a `set`. Perhaps you should double-check whether it's really the right type of container. – Kerrek SB Jan 24 '12 at 05:17
  • 1
    @KerrekSB: `set`? Where do you see a `set`? Been up too late, have you? – Alok Save Jan 24 '12 at 05:24
  • @Als: Oh, sorry. Map, set, same thing... Too drunk to post clearly, I better shut up! – Kerrek SB Jan 24 '12 at 05:25

2 Answers2

3

You have an iterator to the std::map, right? In your for loop, you initialize your iterator to the beginning. You check if it is within bounds. You then erase the iterator. Your iterator is now invalid. The iterators are effectively like pointers and when you erase, you are effectively deleteing a value that was "pointed" to. As such, other iterators can't guarantee that they point to something valid.

Specifically, you erase, causing an invalidation of your iterator. A guaranteed one, because it's the iterator you erased. You need a new iterator, preferably one right after where you erased, right? Unfortunately, std::map does not offer that functionality. std::map isn't really intended to be treated as a sequential container. The idea is that you access values by their key, not their position. Seeing as you're trying to erase by their sequential order, this is problematic.

Instead, perhaps, you could identify which key you need to erase based on i? Then use find to find the key and erase to erase the key. Since you are no longer sequentially iterating through your std::map, you no longer have to worry about invalidated iterators.

Sion Sheevok
  • 4,057
  • 2
  • 21
  • 37
  • This or, y'know, use the appropriate container for whatever it is you're storing. It kind of sounds like `std::map` might not be the right tool for the job. – Sion Sheevok Jan 24 '12 at 05:27
2
int n = nlsize - MAXNL;
while (n-- > 0) {
    LettersMap.erase(LettersMap.begin());
}

The iterator it is becoming instable by erase.

Joop Eggen
  • 107,315
  • 7
  • 83
  • 138