list<class A*> l;

[pub]

When creating a list:
list<class A*> l;

How to delete all the objects whose pointers are contained in "l"?

Thanks for your comments?

 

[Alf P. Steinbach]

When creating a list:
list<class A*> l;

How to delete all the objects whose pointers are contained in "l"?

First iterate through the list and delete each object.

One way to do that is to use a 'for' loop.

Then clear the list.

 

[pub]

First iterate through the list and delete each object.

One way to do that is to use a 'for' loop.

Then clear the list.

Thanks! "iterate through the list" and "delete" every pointer is obvious.
What do you mean by "clear the list"?

 

[Siemel Naran]

Thanks! "iterate through the list" and "delete" every pointer is obvious.
What do you mean by "clear the list"?

Something like

list.clear();

After deleting all the elements in the list you have invalid pointers, so to
be safe and not use them by accident you should remove them. Of course, if
the list is a member of a class and you are doing the for delete each
element in a the destructor, then you don't have to call clear.

Sometimes people do

std::list<A*>().swap(list);

 

[Ivan Vecerina]

When creating a list:
list<class A*> l;

How to delete all the objects whose pointers are contained in "l"?

Given the following utility class:
struct DeleteOp {
template<typename T>
void operator()(const T* p) const { delete p; }
};

You can use on any container of pointers:
std::foreach( l.begin(), l.end(), DeleteOp() );

Thanks for your comments?

you're welcome ??

Ivan

 

[Thorsten Ottosen]

| > When creating a list:
| > list<class A*> l;
| >
| > How to delete all the objects whose pointers are contained in "l"?
|
| First iterate through the list and delete each object.
|
| One way to do that is to use a 'for' loop.
|
| Then clear the list.

This is a *very* bad idea because it is not exception safe. For now, one should
use a container of shared_ptrs like

std::list< boost::shared_ptr<T> >

br

Thorsten

 

[Alf P. Steinbach]

| > When creating a list:
| > list<class A*> l;
| >
| > How to delete all the objects whose pointers are contained in "l"?
|
| First iterate through the list and delete each object.
|
| One way to do that is to use a 'for' loop.
|
| Then clear the list.

This is a *very* bad idea because it is not exception safe.

Do elaborate.

 

[Thorsten Ottosen]

Hi Alf,

| > | > When creating a list:
| > | > list<class A*> l;
| > | >
| > | > How to delete all the objects whose pointers are contained in "l"?
| > |
| > | First iterate through the list and delete each object.
| > |
| > | One way to do that is to use a 'for' loop.
| > |
| > | Then clear the list.
| >
| > This is a *very* bad idea because it is not exception safe.
|
| Do elaborate.

Consider

std::list<T*> l;
l.push_back( new T );
// ... delete pointers later in a loop

this is not exception-safe since a list has no knowlegde about ownership of the stored pointers. l.push_back
can throw bad_alloc if it cannot allocate a node; in that case the stored pointer is never freed. This
problem exists for all types of insertions in standard containers.

However, in

typedef boost::shared_ptr<T> t_ptr;
std::list< t_ptr > l;
l.push_back( t_ptr( new T ) );

the shared_ptr will call delete if push_back fails.

I'm currently working for a set of wrappers for boost that let you store pointers instead of smart pointers in standard
containers and they will be reviewed in August. There are more problems
with standard containers + raw pointers than just insertion so please use shared_ptr<> for now.

HTH

Thorsten

 

[Alf P. Steinbach]

Hi Alf,

| > | > When creating a list:
| > | > list<class A*> l;
| > | >
| > | > How to delete all the objects whose pointers are contained in "l"?
| > |
| > | First iterate through the list and delete each object.
| > |
| > | One way to do that is to use a 'for' loop.
| > |
| > | Then clear the list.
| >
| > This is a *very* bad idea because it is not exception safe.
|
| Do elaborate.

Consider

std::list<T*> l;
l.push_back( new T );
// ... delete pointers later in a loop

this is not exception-safe since a list has no knowlegde about ownership of the stored pointers. l.push_back
can throw bad_alloc if it cannot allocate a node; in that case the stored pointer is never freed.

Well that depends on the context (never say never! ;-)).

But in other words what you meant was exception unsafe was not the procedure
I outlined for deleting the objects, but some specific imagined procedure for
putting the pointers into the list in the first place.

Okay.

 

[Thorsten Ottosen]

| >
| > Consider
| >
| > std::list<T*> l;
| > l.push_back( new T );
| > // ... delete pointers later in a loop
| >
| > this is not exception-safe since a list has no knowlegde about ownership of the stored pointers. l.push_back
| > can throw bad_alloc if it cannot allocate a node; in that case the stored pointer is never freed.
|
| Well that depends on the context (never say never! ;-)).
|
| But in other words what you meant was exception unsafe was not the procedure
| I outlined for deleting the objects, but some specific imagined procedure for
| putting the pointers into the list in the first place.

depends on where you execute your procedure. A lot tends to think

{
list<T*> l; l.push_back( new T );
....
for( iterator i = l.begin(); i != l.end(); ++i ) delete &*i;
}

is ok when its clearly not. Even though you wrap list<T*> in a new class with the for loop in the destructor,
you're still not safe.

br

Thorsten

 

[Siemel Naran]

A lot tends to think

{
list<T*> l; l.push_back( new T );
....
for( iterator i = l.begin(); i != l.end(); ++i ) delete &*i;
}

is ok when its clearly not. Even though you wrap list<T*> in a new class

with the for loop in the destructor,

you're still not safe.

How come it is not safe when you have that code in a destructor?

 

[Siemel Naran]

You can use on any container of pointers:
std::foreach( l.begin(), l.end(), DeleteOp() );

We can also use this idea to implement the copy constructor and operator=.

 

[velthuijsen]

l.push_back( new T );
// ... delete pointers later in a loop

this is not exception-safe since a list has no knowlegde about ownership of the stored pointers. l.push_back
can throw bad_alloc if it cannot allocate a node; in that case the stored pointer is never freed. This
problem exists for all types of insertions in standard containers.

Can't you side step this problem by doing

std::list<T*> l;
T* TempT = new T;
l.push_back(TempT);

Now if l fails to allocate memory you can in the catch block still do
delete TempT

P.s: For anyone who reads this and think ah that is easier then to get
the boost lib. The shared_ptr has a few other handy uses concerning
memory management.

 

[Thorsten Ottosen]

| |
| > A lot tends to think
| >
| > {
| > list<T*> l; l.push_back( new T );
| > ....
| > for( iterator i = l.begin(); i != l.end(); ++i ) delete &*i;
| > }
| >
| > is ok when its clearly not. Even though you wrap list<T*> in a new class
| with the for loop in the destructor,
| > you're still not safe.
|
| How come it is not safe when you have that code in a destructor?

Because every insertion can throw. Erase need to call delete too.
And what about insertion with iterators? Or an iterator range?
Some algorithms cannot be used with list<T*> because they will overwrite
pointers. You need to protect the *whole* interface---which
a fairly big tasks, so use boost::shared_ptr for now.

br

Thorsten

 

[Siemel Naran]

| How come it is not safe when you have that code in a destructor?

Because every insertion can throw. Erase need to call delete too.
And what about insertion with iterators? Or an iterator range?
Some algorithms cannot be used with list<T*> because they will overwrite
pointers. You need to protect the *whole* interface---which
a fairly big tasks, so use boost::shared_ptr for now.

In the copy constructor I have try/catch blocks, and the catch block deletes
the memory allocated in the try block. We need this because if the
constructor throws, the system does not call the destructor (because the
object wasn't fully constructed, so it is not safe to call the destructor).
Then we repeat this desruction code.

So it can be done without boost::shared_ptr, and maybe sometimes it may be a
good idea for performance reasons.

But in general, I'd say go with the shared_ptr.

 

[Alf P. Steinbach]

| >
| > Consider
| >
| > std::list<T*> l;
| > l.push_back( new T );
| > // ... delete pointers later in a loop
| >
| > this is not exception-safe since a list has no knowlegde about ownership of the stored pointers. l.push_back
| > can throw bad_alloc if it cannot allocate a node; in that case the stored pointer is never freed.
|
| Well that depends on the context (never say never! ;-)).
|
| But in other words what you meant was exception unsafe was not the procedure
| I outlined for deleting the objects, but some specific imagined procedure for
| putting the pointers into the list in the first place.

depends on where you execute your procedure.
No.


A lot tends to think

{
list<T*> l; l.push_back( new T );
....
for( iterator i = l.begin(); i != l.end(); ++i ) delete &*i;
}

is ok when its clearly not.

Again it's your imagined procedure for insertion that is unsafe, namely
code that does not properly handle an exception from push_back, and that
has nothing to do with what you answered "not exception safe" to.

The deletion code I sketched or hinted at would be safe.

The code above is not an implementation of that sketch. It's meaningless
Undefined Behavior. Remove the "&" and add clearing of the list after
the loop and the deletion part will be safe under the common assumptions.

Even though you wrap list<T*> in a new class with the for loop in
the destructor, you're still not safe.

That depends very much on the programmer... ;-)

Btw., on style: don't use lowercase ell as a name, because it's so easy to
confuse with the digit one, and do use curly braces around loop bodies.

 

[Thorsten Ottosen]

| > std::list<T*> l;
| > l.push_back( new T );
| > // ... delete pointers later in a loop
| >
| > this is not exception-safe since a list has no knowlegde about ownership of the stored pointers. l.push_back
| > can throw bad_alloc if it cannot allocate a node; in that case the stored pointer is never freed. This
| > problem exists for all types of insertions in standard containers.
|
| Can't you side step this problem by doing
|
| std::list<T*> l;
| T* TempT = new T;

try
{
| l.push_back(TempT);
}
catch( ... ) { delete TempT; }

| Now if l fails to allocate memory you can in the catch block still do
| delete TempT

yes, but this is an awful awful programming style. One should strive for exception neutral code. We don't
want to insert a try catch block, we want automated memory management.

br

Thorsten

 

[Ivan Vecerina]

Can't you side step this problem by doing

std::list<T*> l;
T* TempT = new T;
l.push_back(TempT);

Now if l fails to allocate memory you can in the catch block still do
delete TempT

Rather than a try-catch block, I would use either:
std::auto_ptr<T> item = new T;
l.push_back( item.get() );
item.release();

or eventually:
std::auto_ptr<T> item = new T;
l.push_back( 00 );
l.back() = item.release();

In both cases, the auto_ptr retains the ownership of the item
until a pointer is safely stored in the list. If an exception
is thrown, the allocated object will safely be disposed of.

This said, I agree that an 'ownership-safe' container wrapper
would be nice...

Regards,
Ivan