Crash in erasing element of a list.

[mahajan.vibhor]

I have a list of pointers. e.g

A* a = new A(); // A is a class
stl::list<A*> list_a;

I am inserting object of class in the after allocating memeory thru new
operator.

But when i want to erase all elements from the list. my progam crashes.
I delete element by using a iterator.


A * temp = NULL;
stl::list<A*>::iterator Iter,
stl::list<A*>::iterator IterTemp;
for( Iter = list_a.begin() ; Iter != list_a.end() ; ){
Iter = list_a.begin();
temp = *Iter;
IterTemp = Iter;
Iter++;
list_a.erase(IterTemp);
delete temp;
}

At erase line my program crashes and debugger shows some exception in
erase function.

Any help is greatly appreciated.

Thanks in advance.
Vibhor Mahajan

 

[Noah Roberts]

I have a list of pointers. e.g

A* a = new A(); // A is a class
stl::list<A*> list_a;

I am inserting object of class in the after allocating memeory thru new
operator.

But when i want to erase all elements from the list. my progam crashes.
I delete element by using a iterator.


A * temp = NULL;
stl::list<A*>::iterator Iter,
stl::list<A*>::iterator IterTemp;

for( Iter = list_a.begin() ; Iter != list_a.end() ; ){
Iter = list_a.begin();
temp = *Iter;
IterTemp = Iter;
Iter++;
list_a.erase(IterTemp);
delete temp;
}

Iter is invalid after the erase.

instead of Iter++; list_a.erase() do this:

Iter = list_a.erase(Iter);

 

[Pierre Barbier de Reuille]

Iter is invalid after the erase.

instead of Iter++; list_a.erase() do this:

Iter = list_a.erase(Iter);

Really, when you want to do that kind of things, the *best* way to go
IMHO is :

std::list<A*>::iterator it;
for(it = list_a.begin() ; it != list_a.end() ; ++it)
{
delete *it;
}
list_a.clear();


It will be much faster and much more readable.

Pierre

 

[red floyd]

I have a list of pointers. e.g

A* a = new A(); // A is a class
stl::list<A*> list_a;

I am inserting object of class in the after allocating memeory thru new
operator.

But when i want to erase all elements from the list. my progam crashes.
I delete element by using a iterator.


A * temp = NULL;
stl::list<A*>::iterator Iter,
stl::list<A*>::iterator IterTemp;
for( Iter = list_a.begin() ; Iter != list_a.end() ; ){
Iter = list_a.begin();
temp = *Iter;
IterTemp = Iter;
Iter++;
list_a.erase(IterTemp);
delete temp;
}

At erase line my program crashes and debugger shows some exception in
erase function.

First of all, it should be std::list, not stl::list.

Second, rewrite your delete loop as follows:

for (iter = list_a.begin(); iter != list_a.end ; )
{
delete (*iter);
iter = list_a.erase(iter);
}

 

[red floyd]

Really, when you want to do that kind of things, the *best* way to go
IMHO is :

std::list<A*>::iterator it;
for(it = list_a.begin() ; it != list_a.end() ; ++it)
{
delete *it;
}
list_a.clear();

or even:

// template is more generic
template<typename T>
struct DeletePointer : public std::unary_function<T*,void>
{
void operator()(T* ptr) { delete ptr; }
};

std::for_each(list_a.begin(), list_a.end(), DeletePointer<A>());
list_a.clear();

 

[Roland Pibinger]

I have a list of pointers. e.g

A* a = new A(); // A is a class
stl::list<A*> list_a;

I am inserting object of class in the after allocating memeory thru new
operator.

But when i want to erase all elements from the list. my progam crashes.
I delete element by using a iterator.

Another solution. The pointer to a deleted object is not accessed
after delete (the latter is UB, IIRC):

while (list_a.begin() != list_a.end()) {
A* tmp = list_a.front();
list_a.pop_front();
delete tmp;
}

Best wishes,
Roland Pibinger

 

[red floyd]

while (list_a.begin() != list_a.end()) {
A* tmp = list_a.front();
list_a.pop_front();
delete tmp;
}

Even better:

while (!list_a.empty())
{
A* tmp = list_a.front();
list_a.pop_front();
delete tmp;
}

 

[loufoque]

I have a list of pointers. e.g

A* a = new A(); // A is a class
stl::list<A*> list_a;

That approach has various exception safety issues.
Why not using std::list<A> ? If you need to use pointers for
polymorphism there are alternatives solutions.

 

[Heinz Ozwirk]

First of all, it should be std::list, not stl::list.

Second, rewrite your delete loop as follows:

for (iter = list_a.begin(); iter != list_a.end ; )
{
delete (*iter);
iter = list_a.erase(iter);

Or perhaps even better:

list_a.erase(iter++);

This will work for all STL containers, not just those where erase returns an
iterator to the next element.

Heinz

 

[Mark P]

Or perhaps even better:

list_a.erase(iter++);

This will work for all STL containers, not just those where erase
returns an iterator to the next element.

Depends what you mean by "work". I wouldn't recommend that for a vector.

Mark

 

[Markus Schoder]

Iter is invalid after the erase.

list erase may only invalidate iterators to the erased element. Iter
should still be valid. As has been pointed out the code can be
improved but I see no reason why the given code has undefined behavior.

 

[Noah Roberts]

list erase may only invalidate iterators to the erased element.

I doubt it. Think about what happens:

iterator it = v.begin() + 3;
v.erase(v.begin());

What does it point to now?

Iter

should still be valid. As has been pointed out the code can be
improved but I see no reason why the given code has undefined behavior.

What happens when begin() + 1 == end()?

iterator it = begin();
temp = *it; // ok
iterator tempit = it;
++it; // it == end().
v.erase(tempit); // begin() == end().
delete temp;

end state:

v is empty. it points to begin() + 1. Where is end()? it != end().
Continue the loop...

 

[Old Wolf]

I doubt it.

23.2.2.3 [lib.list.modifiers]

iterator erase(iterator position);
iterator erase(iterator first, iterator last);

Effects: Invalidates only the iterators and references to the
erased elements.

Note - "the iterators" here refers to 'position', 'first', and 'last',
not to all iterators. Compare with the wording for
[lib.deque.modifiers] which states that _all_ iterators and
pointers to the deque are invalidated.

Think about what happens:

iterator it = v.begin() + 3;

operator+ is not defined for std::list.
But you can achieve the same effect by incrementing 3 times.

v.erase(v.begin());

What does it point to now?

The same item it did before.

What happens when begin() + 1 == end()?

iterator it = begin();
temp = *it; // ok
iterator tempit = it;
++it; // it == end().

Note: it == end().

v.erase(tempit); // begin() == end().
delete temp;

end state:
v is empty.
Yes

it points to begin() + 1.

Actually, it == end(), as noted above.

The expression "begin() + 1" is meaningless, as operator+ is
not defined for lists. I guess you mean 'the element after begin()',
but that isn't where it is pointing.

Continue the loop...

If your loop now increments 'it', then it should be considered
a bug in the loop code.

 

[Noah Roberts]

I doubt it.

23.2.2.3 [lib.list.modifiers]

Ok, I was talking about vectors and the OP wanted to know about lists;
my **** up. There is no 'problem' in the OP's shown code even if it
isn't the 'best' way to do it. The crash is caused elsewhere. The OP
should read the faq and provide the minimal code that compiles and
exhibits the problem.