STL list::erase()

[s]

Here is a snippet of my code:

<code>

list< MyClass * >outgoing_pool;

MyClass* GetWaitingObject(string freq)
{
MyClass *temp_ptr = NULL;
list<MyClass*>::iterator i;

for(i=outgoing_pool.begin(); i!=outgoing_pool.end(); i++)
{
if( (*i)->GetFrequency() == freq ) //match
{
temp_ptr = *i;
//remove item from list
outgoing_pool.erase(i);
}
}

return temp_ptr;
}

</code

This function is supposed to iterate through the list and remove a match
(if found). However, I'm getting a segmentation fault on the
(*i)->GetFrequency() call. This only happens after an element has been
erase()'ed out of "outgoing_pool". Does the iterator i need to be
"updated" or some such after a call to erase? I was thinking that it
was smart enough to handle this but now I'm not so sure...

I guess my question is (not totally related to the above code, but...)
what is a good way to iterate through a std::list and remove selected
elements?

Thanks,
S

[Andre Kostur]

s <youshouldbe@home> wrote in news:3F660A7A.4060507@home:

Here is a snippet of my code:

<code>

list< MyClass * >outgoing_pool;

MyClass* GetWaitingObject(string freq)
{
MyClass *temp_ptr = NULL;
list<MyClass*>::iterator i;

for(i=outgoing_pool.begin(); i!=outgoing_pool.end(); i++)
{
if( (*i)->GetFrequency() == freq ) //match
{
temp_ptr = *i;
//remove item from list
outgoing_pool.erase(i);
}
}

return temp_ptr;
}

</code

This function is supposed to iterate through the list and remove a match (if found). However, I'm getting a segmentation fault on the
(*i)->GetFrequency() call. This only happens after an element has been
erase()'ed out of "outgoing_pool". Does the iterator i need to be
"updated" or some such after a call to erase? I was thinking that it
was smart enough to handle this but now I'm not so sure...
Logic question... can only one item in your list have the specified
frequency?

But... that's not your problem. Your problem is in your for loop. After
calling outgoing_pool.erase(i), your iterator becomes invalid. The next
thing that your for loop will do is attempt to increment your iterator.
Since it's invalid, that invokes undefined behaviour (which in your case
means segfault). You'll need some logic to control your increment when
you erase your iterator to prevent the usage of an invalid iterator.

Actually, it also depends on the answer to my logic question above. If
there can only be one object that will match, you could just "return
temp_ptr;" right after the erase (although some people will complain
about multiple returns from a function), or add a "break" statement to
break out of the for loop.

If there can be multiple objects with the same frequency, you've got a
bigger problem :) Looks like you would attempt to erase them all
(assuming the iterator problem is fixed), but only return the first one
to your caller.....
I guess my question is (not totally related to the above code, but...)
what is a good way to iterate through a std::list and remove selected
elements?

You could do some stuff with std::list::remove_if.....

[Rob Williscroft]

s wrote in news:3F660A7A.4060507@home:

Here is a snippet of my code:

<code>

list< MyClass * >outgoing_pool;

MyClass* GetWaitingObject(string freq)
{
MyClass *temp_ptr = NULL;
list<MyClass*>::iterator i;

for(i=outgoing_pool.begin(); i!=outgoing_pool.end(); i++)

for(i=outgoing_pool.begin(); i!=outgoing_pool.end(); )
{
if( (*i)->GetFrequency() == freq ) //match
{
temp_ptr = *i;
//remove item from list
outgoing_pool.erase(i);
outgoing_pool.erase(i++);
}
else ++i;
}

return temp_ptr;
}

</code

This function is supposed to iterate through the list and remove a
match (if found). However, I'm getting a segmentation fault on the
(*i)->GetFrequency() call. This only happens after an element has
been erase()'ed out of "outgoing_pool". Does the iterator i need to
be "updated" or some such after a call to erase? I was thinking that
it was smart enough to handle this but now I'm not so sure...

It needs to be updated before the call to erase(), hence the i++
and the else ++i in the above code.

HTH

Rob.
--
http://www.victim-prime.dsl.pipex.com/