cppcheck error : Dangerous iterator usage

Question

The code:

for(x=abc.begin();x!=abc.end();x++)  
{  
   if(-----)  
   {
      ----  
      abc.erase(x);  
   }
}

And the error is :::
Dangerous iterator usage
After erase the iterator is invalid so dereferencing it or comparing it with another iterator is invalid.

what is the wrong usage in using erase function in the above code?

harper · Accepted Answer · 2011-03-14T09:07:10.593

7

The itarator x is invalid after deleting the corresponding value from abc. This should fix it:

x = abc.begin();

while(x != abc.end())
{
    if (-----)
    {
        ----
        x = abc.erase(x);
        // skipped only to next item
    }
    else
    {   // skip only to next item
        ++x;
    }
}

The erase template functions of STL containers return the next element, or end().

Edit: Thanks for comment by templatetypedef.

edited Mar 14 '11 at 09:07

answered Mar 14 '11 at 07:21

harper

13,345
8
56
105

1

This correctly avoids invalidation, but be careful... you'll end up skipping the `x` iterator past the elements right after those that get erased. – templatetypedef Mar 14 '11 at 07:26

score 5 · Answer 2 · answered Mar 14 '11 at 06:22

5

You're using x as the control variable in the loop. Since it is invalidated by erase(), you cannot be sure that it is safe (or meaningful) to subsequently increment it at the top of the loop.

answered Mar 14 '11 at 06:22

Stabledog

3,110
2
32
43

score 4 · Answer 3 · answered Mar 14 '11 at 06:22

4

x is a pointer into abc. Once you've erased the item pointed to by x, what is x supposed to be pointing to and how is x++ supposed to work?

answered Mar 14 '11 at 06:22

geekosaur

59,309
11
123
114

score 1 · Answer 4 · answered Mar 14 '11 at 09:57

You said nothing about the container you are iterating on. On the type of the container depends which iterators are invalidated. For sure iterator to erased element is invalid, but for example in std::vector all iterators past erased element will be invalid (including end()). And for unknown reason although set::erase invalidates only iterator to erased element, it does not return the iterator to next element.

So with std::set:

while (x != abc.end()) // end() will not change and even can be stored
{
    if (...)
        abc.erase(x++); // increments before erasing
    else
        ++x;
}

cppcheck error : Dangerous iterator usage

4 Answers4