CODEWITHSUNDEEP
c++iteratorstdstdvector
Wojtek
Wojtek

Reputation: 2524

Vector gets iterated more times than size()

I've got this piece of code:

for (std::vector<Marker>::iterator it = markers.begin(); it != markers.end(); ++it) {
    if (it->getDots().size() < 3) {
        markers.erase(it);
    }
}

In one of test inputs (the app does image analysis) I get a segfault. I tried to debug the code (to no avail) and noticed one thing. When asking gdb to p markers.size() i receive $9 = 3. So I would expect the loop to iterate three times, but surprisingly it does it (at least) 5 times. In fifth iteration there's a segfault. What I also noticed is that it's not the dereference of *it (here it->) that causes the error. It's specifically it->getDots(), which is a simple getter.

I write in C++ very rarely, so it might be some simple mistake, but neither my debugging, nor googling brought any solution. Could you help?

I'd like to emphasize, that on various different inputs (slightly different images) this function works properly, so it's even harder for me to trace the bug down.

Upvotes: 2

Views: 101

Answers (3)

Mats Petersson
Mats Petersson

Reputation: 129524

You need to update it when you erase:

it = markers.erase(it);

since erase will "change" the vector, and your current it is no longer valid (and only do it++ if when you didn't erase it).

However, the more common way to do this is to use this type of construction:

markers.erase(std::remove(markers.begin(), markers.end(), number_in), markers.end());

Upvotes: 2

Igor Tandetnik
Igor Tandetnik

Reputation: 52621

vector::erase invalidates all iterators pointing to the element being erased, and all elements that follow. So it becomes invalid, and ++it expression on the next loop iteration exhibits undefined behavior.

The best way to code this logic is with erase-remove idiom.

Upvotes: 6

nikolas
nikolas

Reputation: 8975

The problem is this line:

markers.erase(it);

The iterator is invalidated. But that's okay, erase returns a valid iterator:

auto it = markers.begin();
while(it != markers.end())
{
    if(it->getDots().size() < 3) {
        it = markers.erase(it);
    }
    else ++it;
}

Upvotes: 3

Related Questions