C++ loop on map not detecting change of map`s end

Question

I am having a problem while looping thru a map (std::map).

Inside my loop, there is a call to a function which sometimes (not always) erases elements of this same map. After this function is used, there is some code which is using some of this map information as input.

I am having no problems after this function erases any elements, except on the unique case that the last element of the map is erased.

My loop semms not to understand that the last element of the map is not the same as when it started to operate, and will try to operate on elements which doesnt exist, creating a crash.

It seems to me that the myMap.end() call on the loop description is not able to update itself with the new end() of the map.

The relevant part of the code is listed below:

for(std::map<int, ConnectionInfo>::iterator kv = myMap.begin(); kv != myMap.end(); ++kv) {
        int thisConnectionID=kv->first; //This is where I get garbage when the loop enters when it shouldnt;
        ConnectionInfo currentConnectionInfo=kv->second; //This is where I get garbage when the loop enters when it shouldnt;
        status=eraseSomeMapElementsIfNecessary(thisConnectionID,currentConnectionInfo.DownPacket); //this function might erase elements on myMap. This generates no problems afterwards, except when the end element of myMap is erased
        ... //Next parts of the code make no further usage of myMaps, so I just hid it not to pollute the code
}

Is my interpretation that the kv != myMap.end() is not being able to understand that the inner loop is changing (erasing) the last element (end) of myMap?

In this case, how can I fix this issue?

Or is my interpretation wrong and the solution has nothing to do with what I stated before?

Thanks for your help!

Answer 1

The usual idiom when iterating a map with possibly deleting element is:

for(auto it = map.begin(); it != map.end(); ) {
   if ( *it == /*is to delete*/ ) {
     it = map.erase(it);
   }
   else
     ++it;
}

if your eraseSomeMapElementsIfNecessary might erase some random values in map being iterated then this will for sure cause problems. If element to which it is referencing was erased, it becomes invalid, then incrementing it with ++it is also invalid.

The problem is actually only with the it iterator, if eraseSomeMapElementsIfNecessary erases it and then you use it - you have Undefined Behaviour (UB). So the solution is to pass current iterator to eraseSomeMapElementsIfNecessary, and return from it the next one to iterate:

it = eraseSomeMapElementsIfNecessary(it);

the body of the for loop from my example should be inside your eraseSomeMapElementsIfNecessary function. At least this is one solution.