How to remove item from STL list

[sam]

Hi,

I don't know the correct way to remove an item from a STL list.
The following codes I supposed it works, but generated run-time coredump:

for (l_iter=l_peer_info.begin(); l_iter!=l_peer_info.end(); l_iter++) {
for (m_iter=l_iter->begin(); m_iter!=l_iter->end(); m_iter++) {
int ret = rxx.exec(m_iter->first, macro_parser);
if (ret) {
//(*l_iter).erase(m_iter);
l_peer_info.erase(l_iter);
break;
//continue;
}
....

the result of running the above code is shown error as below:
in free(): error: junk pointer, too high to make sense
Abort (core dumped)

The second method was to remove the commend of the above code and
comment out the uncommented code.


Thanks
Sam

 

[Rolf Magnus]

Hi,

I don't know the correct way to remove an item from a STL list.
The following codes I supposed it works, but generated run-time coredump:

for (l_iter=l_peer_info.begin(); l_iter!=l_peer_info.end(); l_iter++) {
for (m_iter=l_iter->begin(); m_iter!=l_iter->end(); m_iter++) {
int ret = rxx.exec(m_iter->first, macro_parser);
if (ret) {
//(*l_iter).erase(m_iter);
l_peer_info.erase(l_iter);
break;
//continue;
}
...

the result of running the above code is shown error as below:
in free(): error: junk pointer, too high to make sense
Abort (core dumped)

That's because the iterator is invalidated by the erase() function. Note
that erase() returns an iterator to the element following the erased one.

 

[Mike Wahler]

Hi,

I don't know the correct way to remove an item from a STL list.
The following codes I supposed it works, but generated run-time coredump:

for (l_iter=l_peer_info.begin(); l_iter!=l_peer_info.end(); l_iter++) {
for (m_iter=l_iter->begin(); m_iter!=l_iter->end(); m_iter++) {
int ret = rxx.exec(m_iter->first, macro_parser);
if (ret) {
//(*l_iter).erase(m_iter);
l_peer_info.erase(l_iter);
break;
//continue;
}
...

the result of running the above code is shown error as below:
in free(): error: junk pointer, too high to make sense
Abort (core dumped)

The second method was to remove the commend of the above code and comment
out the uncommented code.

'std::list::erase()' invalidates the iterator given as its
argument. It returns an iterator to the next element in the
list, or 'end()' if no such element exists. The way you have
it, after erasing an element, your 'for' loop tries to increment
an invalid iterator, leading to Bad Things(tm). Try:

l_iter = l_peer_info.erase(l_iter);

When erasing elements from any container, you should check the
documentation for if/how/when/which iterators are still valid
and which are not (this depends upon which container type you
use).

-Mike

 

[Kristo]

Hi,

I don't know the correct way to remove an item from a STL list.
The following codes I supposed it works, but generated run-time
coredump:

[indenting changed a bit to fit in 72 chars]

for (l_iter=l_peer_info.begin(); l_iter!=l_peer_info.end(); l_iter++)
{
for (m_iter=l_iter->begin(); m_iter!=l_iter->end(); m_iter++) {
int ret = rxx.exec(m_iter->first, macro_parser);
if (ret) {
//(*l_iter).erase(m_iter);
l_peer_info.erase(l_iter);
break;
//continue;
}
...

As stated elsethread, erase will invalidate iterators, thus breaking
your loop. A better solution would be to write a predicate that you
could pass to the list member function remove_if. This is different
from std::remove_if in that it actually removes elements. It will
remove all elements that match the predicate, thus eliminating the need
for your loops.

Kristo

 

[Kristo]

...
write a predicate that you could pass to the list member function
remove_if. This is different from std::remove_if in that it actually
removes elements.

^^^^^^^^^^^^^^^^

I realized that this might be a bit confusing. All versions of remove
and remove_if will destroy the elements being "removed." The versions
that are member functions of std::list will also shorten the length of
the list, whereas the free versions will not.

Kristo

 

[Mark P]

^^^^^^^^^^^^^^^^

I realized that this might be a bit confusing. All versions of remove
and remove_if will destroy the elements being "removed." The versions
that are member functions of std::list will also shorten the length of
the list, whereas the free versions will not.

Kristo

This issue is nicely discussed in Meyers's Effective STL.

 

[Rolf Magnus]

'std::list::erase()' invalidates the iterator given as its
argument. It returns an iterator to the next element in the
list, or 'end()' if no such element exists. The way you have
it, after erasing an element, your 'for' loop tries to increment
an invalid iterator, leading to Bad Things(tm). Try:

l_iter = l_peer_info.erase(l_iter);

I wanted to suggest that at first, but that's a bit problematic, because
afterwards, l_iter gets incremented by the for loop. This means two things.
First, after each erased element, one element gets ignored. Second, if
erase() returns end(), you get the Bad Things(tm) again.

When erasing elements from any container, you should check the
documentation for if/how/when/which iterators are still valid
and which are not (this depends upon which container type you
use).

But I'd say it's pretty sure that any iterators to erased elements will be
invalidated, right?

 

[sam]

Mike Wahler wrote:

After added the following code in the second for loop, it works without
skip the next item:
l_iter = l_peer_info.erase(l_iter);
l_iter--;

 

[Mike Wahler]

I wanted to suggest that at first, but that's a bit problematic, because
afterwards, l_iter gets incremented by the for loop. This means two
things.
First, after each erased element, one element gets ignored. Second, if
erase() returns end(), you get the Bad Things(tm) again.

You're right. I didn't look very closely at the code as a whole.
It would need to be changed to e.g. a 'while' loop, roughly:

while(l_iter!=l_peer_info.end())
{
if(condition)
l_iter = l_peer_info.erase(l_iter);
else
++l_iter;

}

Thanks for pointing out my mistake.

But I'd say it's pretty sure that any iterators to erased elements will be
invalidated, right?

Right, until such time as 'phantom iterators' are invented.

-Mike

 

[Mike Wahler]

After added the following code in the second for loop, it works without
skip the next item:
l_iter = l_peer_info.erase(l_iter);
l_iter--;

Careful! This will give a different problem. What
happens if the first element is erased?

See my reply to Rolf for a possible solution.

-Mike

 

[sam]

Careful! This will give a different problem. What
happens if the first element is erased?

When the first item is removed, the second item becomes the first, and
the l_iter automatically advanced to the next one, so it skips one item
(the original second item, and now becomes the first item).

Therefore the l_iter-- must be executed in this regard (as calling erase()).

If there is another cleaner way to handle this, please continue the
discussion.

Thanks
Sam.

 

[sam]

You're right. I didn't look very closely at the code as a whole.
It would need to be changed to e.g. a 'while' loop, roughly:

while(l_iter!=l_peer_info.end())
{
if(condition)
l_iter = l_peer_info.erase(l_iter);
else
++l_iter;

}

Yeah, I think a while loop is a proper way to handle erash() function
call in a list. With a For loop, it will skip one item.

Sam.

 

[Mike Wahler]

When the first item is removed, the second item becomes the first,

Right. But right after the erase, you have
l_iter-- , which will try to decrement the iterator value
which is equal to 'begin()'. Not Good.

the l_iter automatically advanced to the next one,

That happens too late.

so it skips one item (the original second item, and now becomes the first
item).

Therefore the l_iter-- must be executed in this regard (as calling
erase()).

See above.

If there is another cleaner way to handle this, please continue the
discussion.

Try the 'while' loop solution in my other post.

-Mike