CODEWITHSUNDEEP
c++visual-c++
user1107855
user1107855

Reputation: 345

map/set iterator not incrementablemap/set iterator not incrementable

Driver::~Driver()
{
    AutoCritSec acsDriverList(m_csDriverList,true);
    DRIVERLIST::iterator it = m_DriverList.begin();
    for(;it!=m_DriverList.end();it++) 
    {
        if (it->second == this) 
        {
            m_DriverList.erase(it);
            it = m_DriverList.begin();
        }
    }
}

When I compile my program in visual studio 2003, my program behaves well and good. but when i do the same in 2010, then while closing the application i get some error like 

Expression:map/set iterator not incrementable

and when i press to ignore this i get

Expression:"standard c++ library out of range" && 0

Does any one has any idea what is going on here: I will extremely indebted for any suggestions by anyone. Tons of thanks and warm wishes.

Upvotes: 16

Views: 20636

Answers (2)

Kerrek SB
Kerrek SB

Reputation: 477060

The correct erase idiom for associative containers is as follows:

for (auto it = container.begin(); it != container.end() /* not hoisted */; /* no inc. */ )
{
    if (delete_condition)
    {
        container.erase(it++);
    }
    else
    {
        ++it;
    }
}

Upvotes: 14

James McNellis
James McNellis

Reputation: 355069

If this is the only element in the list, you will overrun the end of the list.

After you remove this from the list, you reset it = m_DriverList.begin();. This is fine. Then the loop expression is evaluated (the i++ from the for statement), which causes it to be advanced past the end of the range.

Advancing an iterator past the end of the container causes the program to exhibit undefined behavior. Recent versions of Visual C++ helpfully detect many common iterator errors in debug builds of your program and raise assertions to help you to solve them.

You can resolve the problem by removing loop expression and moving it into an else statement:

while (it != m_DriverList.end())
{
    if (it->second == this)
    {
        m_DriverList.erase(it);
        it = m_DriverList.begin();
    }
    else
    {
        ++it;
    }
}

Though, restarting iteration every time you remove an element is rather wasteful. Consider instead using using the iterator returned by the call to erase:

it = m_DriverList.erase(it);

Upvotes: 16

Related Questions