Segmentation fault with STL list class - PLEASE HELP

[cpptutor2000]

I am using an STL list to store data packets in a network simulator.
The data packets are really C structs, which have other C structs
inside them. These structs contain
unsigned char arrays of various sizes, and some unsigned int values.
There are no pointers values stored in these structs.
I have a function, from inside of which I have a loop as follows:
Please note that dataPacketStore is the name of the struct being
stored.

if(!dlist.empty()){
list<dataPacketStore>::iterator iter;
list<dataPacketStore>::iterator dbegin = dlist.begin();
list<dataPacketStore>::iterator dend = dlist.end();

for(iter = dbegin; iter != dend; iter++)
if((*iter).disabled == true) dlist.erase(iter);
//'disabled' is a boolean variable in the struct dataPacketStore
}

Calling this erase method causes a segmentation fault. Is this because
of the fact that nested structs are there? Any help or suggestions
would be greatly appreciated.
Thanks in advance for your help.

 

[Thomas Tutone]

I am using an STL list to store data packets in a network simulator.
[snip]

I have a function, from inside of which I have a loop as follows:
Please note that dataPacketStore is the name of the struct being
stored.

if(!dlist.empty()){
list<dataPacketStore>::iterator iter;
list<dataPacketStore>::iterator dbegin = dlist.begin();
list<dataPacketStore>::iterator dend = dlist.end();

for(iter = dbegin; iter != dend; iter++)
if((*iter).disabled == true) dlist.erase(iter);

Calling list's erase member function invalidates all iterators that
point to the value that was removed. So, in your example, you erase
the element pointed to by iter, which invalidates iter (think about it
- it points to an element that has been removed from the list). Then
you try to increment iter. That leads to undefined behavior (which can
include seg faults). But erase actually returns a valid iterator. Try
this instead (untested):

if (iter->disabled) iter = dlist.erase(iter);

Best regards,

Tom

 

[Greg]

I am using an STL list to store data packets in a network simulator.
[snip]

I have a function, from inside of which I have a loop as follows:
Please note that dataPacketStore is the name of the struct being
stored.

if(!dlist.empty()){
list<dataPacketStore>::iterator iter;
list<dataPacketStore>::iterator dbegin = dlist.begin();
list<dataPacketStore>::iterator dend = dlist.end();

for(iter = dbegin; iter != dend; iter++)
if((*iter).disabled == true) dlist.erase(iter);

Calling list's erase member function invalidates all iterators that
point to the value that was removed. So, in your example, you erase
the element pointed to by iter, which invalidates iter (think about it
- it points to an element that has been removed from the list). Then
you try to increment iter. That leads to undefined behavior (which can
include seg faults). But erase actually returns a valid iterator. Try
this instead (untested):

if (iter->disabled) iter = dlist.erase(iter);

Best regards,

Tom

Your suggestion will fix the crash, but it has introduced another, more
subtle problem.

The loop will now skip over the iterator that erase() returned, by
incrementing it before the test is next evaluated. For every item
deleted (unless it's the last in the list) another item in the list
will be skipped.

Greg

 

[Earl Purple]

which is why you should probably prefer to use the algorithmic

dlist.erase( std::remove_if( dbegin, dend, isDisabled ) );

where isDisabled is defined as:

bool isDisabled ( const dataPacketStore & dps )
{
return dps.disabled;
}

and possibly you could use a function object:

struct IsDisabled
{
bool operator() ( const dataPacketStore & dps ) const
{
return dps.disabled;
}
};

in which case change the algorithmic call to

dlist.erase( std::remove_if( dbegin, dend, IsDisabled() ) );

 

[Earl Purple]

oops too late I can't go back and retrieve my message. But you'll need
another dend in the erase thus:

dlist.erase( std::remove_if( dbegin, dend, isDisabled ), dend );

and

dlist.erase( std::remove_if( dbegin, dend, IsDisabled() ), dend );

 

[Earl Purple]

actually no need for that at all as list has its own remove_if function
thus:

dlist.remove_if( dbegin, dend, isDisabled );

or

dlist.remove_if( dbegin, dend, IsDisabled() );

The other method will work but using list's own remove_if is preferred.

 

[Clark S. Cox III]

oops too late I can't go back and retrieve my message. But you'll need
another dend in the erase thus:

dlist.erase( std::remove_if( dbegin, dend, isDisabled ), dend );

and

dlist.erase( std::remove_if( dbegin, dend, IsDisabled() ), dend );

Or just:

dlist.remove_if(isDisabled)

and

dlist.remove_if(IsDisabled());

 

[Thomas Tutone]

Your suggestion will fix the crash, but it has introduced another, more
subtle problem.

The loop will now skip over the iterator that erase() returned, by
incrementing it before the test is next evaluated. For every item
deleted (unless it's the last in the list) another item in the list
will be skipped.

Absolutely right. Thanks for the correction.

I suppose that one could do something like:

while (iter!=dlist.end() && iter->disabled) iter = dlist.erase(iter);

But I agree with another poster's suggestion that one is better off
using an algorithm-like member function instead.

Best regards,

Tom

 

[Clark S. Cox III]

Absolutely right. Thanks for the correction.

I suppose that one could do something like:

while (iter!=dlist.end() && iter->disabled) iter = dlist.erase(iter);

This, of course, would stop at the first enabled object:

(D=Disabled, E=Enabled)
Before: (DDDDEDED)
After: (EDED)