std::vector<A*> my_vector;
Now, after using this vector, I need to destroy all the memory I have
allocated so I do a for_each over all the pointers and delete each
one with:
std::for_each(my_vector.begin(), my_vector.end(), ptr_fun(&::operator
delete));
This doesn't seem to actually call the destructor for A. Is there
some problem with the way that I am using the ptr_fun adaptor? The
only way that I have found to solve this problem is create a wrapper
functor that does the same thing and use that as the parameter to
for_each:
template <class T>
struct delete_ptr : std::unary_function<T*, void>
{
void operator() (T* p) const
{
delete p;
}
};
std::for_each(my_vector.begin(), my_vector.end(), delete_ptr<A>());
Is there a more standard way of doing this? Perhaps even in something
semi-standard such as Boost?
Mike Gibson
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
This line contains various syntax errors, but I think I can guess what
you
want to do.
It's a big No, No.
::operator delete only frees memory. It does not call any destructors.
>
> This doesn't seem to actually call the destructor for A. Is there
> some problem with the way that I am using the ptr_fun adaptor?
You use the wrong function to be called.
The
> only way that I have found to solve this problem is create a wrapper
> functor that does the same thing and use that as the parameter to
> for_each:
>
> template <class T>
> struct delete_ptr : std::unary_function<T*, void>
> {
> void operator() (T* p) const
> {
> delete p;
> }
> };
>
> std::for_each(my_vector.begin(), my_vector.end,delete_ptrA
More correct than your previous try, but still wrong (besides the
obvious
syntax errors).
std::vector requires its instantiation type to be copyable. At all
times.
When you call delete p, then the pointer remaining in the vector is not
copyable any more.
The solution to this problem would be to pass the pointer per reference
and
set it to 0 after the call to delete.
However, there is left the problem with exception safety. So you go on
to:
>
> Is there a more standard way of doing this? Perhaps even in something
> semi-standard such as Boost?
Yes. It's called shared_ptr. You instantiate your container with this
smart
pointer, and not with dumb pointers.
Alternatively, you can look for libraries that contain smart containers
invented for exactly this reason (to manage pointers to heap-objects).
Google will help you.
Thomas
Presumably, each element of my_vector was individually allocated with
"new A" and nobody else is using or deleting these. Frankly, the
proper way would NOT be tossing around pointers that need to be managede
seperately, but to wrap the dynamic allocation of A inside some other
class that would manage it (perhaps a boost shared_ptr).
>
> Now, after using this vector, I need to destroy all the memory I have
> allocated so I do a for_each over all the pointers and delete each
> one with:
>
> std::for_each(my_vector.begin(), my_vector.end(), ptr_fun(&::operator
> delete));
Despite it's unfortunate name, "operator delete" is NOT the
implementation
of the delete operator, it's the memory deallcoator. Not only does the
above not call the destructors, it isn't even necesarily the way to
deallocate the memory (perhaps A used some other allocation function).
My advoce is that what you're are doing is going to be considerable
harder
than you think. For example, all member functions must be made
exception-safe
to
prevent all leaks.
| Is there a more standard way of doing this? Perhaps even in something
| semi-standard such as Boost?
We just had a review of my Smart Container Library...it probably does
what you
want.
You can download the review version from
http://groups.yahoo.com/group/boost/files/pointer%20container/
smart_container.zip
Feedback is welcome
br
Thorsten
The behaviour you experience is correct. "operator delete" simply frees
the storage and it's not supposed to invoke the destructor. It's the
delete-expression, i.e.: "delete p", that first invokes the destructor
and subsequently calls operator delete.
> The
> only way that I have found to solve this problem is create a wrapper
> functor that does the same thing and use that as the parameter to
> for_each:
>
> template <class T>
> struct delete_ptr : std::unary_function<T*, void>
> {
> void operator() (T* p) const
> {
> delete p;
> }
> };
>
> std::for_each(my_vector.begin(), my_vector.end(), delete_ptr<A>());
That's what I would write. Some people suggested that it should be more
correct to write:
template <class T>
struct delete_ptr : std::unary_function<T*, void>
{
void operator() (T*& p) const
{
delete p;
p = 0;
}
};
That's because leaving invalid pointers into a container may trigger
undefined behaviour on certain implementations, even if the cointer is
going to be destroyed right away. Personally, I never worked on an
implementation where that would really make a difference.
>
> Is there a more standard way of doing this? Perhaps even in something
> semi-standard such as Boost?
>
In Boost there is a functional boost::checked_deleter<T> that does
exactly what your delete_ptr<T> does, plus a safety-check that the type
is actually complete (deleting an incomplete type may trigger undefined
behaviour).
HTH,
Alberto
Don't! Use a vector of smart pointers instead.
> Now, after using this vector, I need to destroy all the memory I have
> allocated so I do a for_each over all the pointers and delete each
> one with:
>
> std::for_each(my_vector.begin(), my_vector.end(), ptr_fun(&::operator
> delete));
>
> This doesn't seem to actually call the destructor for A. Is there
> some problem with the way that I am using the ptr_fun adaptor?
No, the problem is that '::operator delete' is only responsible for
releasing memory, it is NOT the same as invoking delete on a pointer.
> The only way that I have found to solve this problem is create a
> wrapper functor that does the same thing and use that as the parameter
> to for_each:
>
> template <class T>
> struct delete_ptr : std::unary_function<T*, void>
> {
> void operator() (T* p) const
> {
> delete p;
> }
> };
>
> std::for_each(my_vector.begin(), my_vector.end(), delete_ptr<A>());
>
> Is there a more standard way of doing this?
I don't think so.
> Perhaps even in something semi-standard such as Boost?
Boost has something called checked_delete(r), which is basically the same as
your delete_ptr<> but it additionally assures that T is a complete type(you
don't want to call delete on a pointer to an incomplete type).
Uli
--
FAQ: http://parashift.com/c++-faq-lite/
/* bittersweet C++ */
default: break;
> std::vector<A*> my_vector;
> Now, after using this vector, I need to destroy all the memory I have
> allocated so I do a for_each over all the pointers and delete each one
> with:
> std::for_each(my_vector.begin(), my_vector.end(), ptr_fun(&::operator
> delete));
> This doesn't seem to actually call the destructor for A.
Normal. You're calling the deallocation function. There is no function
for the operator delete.
> Is there some problem with the way that I am using the ptr_fun
> adaptor? The only way that I have found to solve this problem is
> create a wrapper functor that does the same thing and use that as the
> parameter to for_each:
> template <class T>
> struct delete_ptr : std::unary_function<T*, void>
> {
> void operator() (T* p) const
> {
> delete p;
> }
> };
> std::for_each(my_vector.begin(), my_vector.end(), delete_ptr<A>());
> Is there a more standard way of doing this?
Only what you're doing. Except that technically, what you're doing is
undefined behavior. It's formally illegal for a container to hold a
delete pointer, so you have to do something like:
void operator()( T*& p )
{
T* tmp = NULL ;
std::swap( tmp, p ) ;
delete tmp ;
}
In practice, I wouldn't worry about it.
> Perhaps even in something semi-standard such as Boost?
Not that I know of, although there is boost::bind.
--
James Kanze GABI Software http://www.gabi-soft.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
If you'd put smart pointers in the vector, the pointers would handle
the deletion for you when they're destroyed by the vectors DTOR.
Other than that, why not
while (!myvector.empty()) {
delete myvector.back();
myvector.pop_back();
}
Has the advantage of not leaving dangling pointers in the vector (ok,
the vector will cease to exist right after this, but...)
R'
--- 8< ---
> template <class T>
> struct delete_ptr : std::unary_function<T*, void>
> {
> void operator() (T* p) const
> {
> delete p;
> }
> };
>
> std::for_each(my_vector.begin(), my_vector.end(), delete_ptr<A>());
>
I've been adviced a nicer version:
struct delete_ptr
{
template <class P>
void operator() (P p)
{
delete p;
}
};
std::for_each(my_vector.begin(), my_vector.end(), delete_ptr());
Note that you don't have to specify the type of the objects you want to
delete anymore.
Didier
> As is commonly done, I have created a container of pointers to objects
> of class A:
>
> std::vector<A*> my_vector;
>
> Now, after using this vector, I need to destroy all the memory I have
> allocated
Actually, you have to destruct the objects pointed to by my_vector's
elements. Deallocating the memory they occupy is only part of the story.
> so I do a for_each over all the pointers and delete each
> one with:
>
> std::for_each(my_vector.begin(), my_vector.end(), ptr_fun(&::operator
> delete));
This causes ::operator delete() to invoked for each of my_vector's elements.
::operator delete() is a mere deallocation function.
> This doesn't seem to actually call the destructor for A.
This is the consequence.
> Is there some problem with the way that I am using the ptr_fun adaptor?
No. The problem is with the approach.
> The
> only way that I have found to solve this problem is create a wrapper
> functor that does the same thing and use that as the parameter to
> for_each:
>
> template <class T>
> struct delete_ptr : std::unary_function<T*, void>
> {
> void operator() (T* p) const
> {
> delete p;
> }
> };
>
> std::for_each(my_vector.begin(), my_vector.end(), delete_ptr<A>());
>
> Is there a more standard way of doing this? Perhaps even in something
> semi-standard such as Boost?
Change the value type of my_vector:
std::vector< boost::shared_ptr<A> > my_vector;
This makes my_vector's elements the true owners of "their" A object.
The A objects they point to will be reliably destructed, e.g. when
- my_vector itself is destructed
- my_vector.clear() is called
- elements are erased from my_vector
I use two extremely usefull templates for most of my memory deallocation
needs.
Firs there is the element delter which is basically the same as the above
except that it also zeros the pointer (on demand or always for debugging)
which
can be of great help because using a 0 pointer gives segfault and not strange
behaviour.
template<typename T, bool zero=false>
class ElemDeleter {
public:
void operator()(T* &o) {
delete o;
#ifdef NDEBUG
if(zero)
#endif
o = 0;
}
};
Following the same line of thought I usually also have a
delete-and-set-to-NULL
function that I use for deletion of "normal" pointers.
template<typename T>
inline void DelNZero(T* &o) { delete o; o = NULL; }
Just don't forget that if you are working with arrays you need special
functions for them to use the correct delete[] operator.
Here I also assume the compiler is smart enough to optimize the if(false) away
for template argument (since it is known at compiletime) so the zeroing in
ElemDeleter is not any overhead if you don't use it.
Anyone care to comment on this, any insight if this is good or bad practice,
any issues I havn't thought of?
/Daniel Aarno
Why not a shorter version:
template <typename T>
void delete_pointer( T* p)
{
delete p;
}
for_each( my_vector.begin(), my_vector.end(), delete_pointer);
Catalin
Correct me if I'm wrong, but a pointer is copyable no matter what it
points to. The container never dereference the pointer.
__
Jonas Norberg
3.7.3.2/4: "If the argument given to a deallocation function in the
standard library is a pointer that is not the null pointer value
(4.10), the deallocation function shall deallocate the storage
referenced by the pointer, rendering invalid all pointers referring to
any part of the deallocated storage. The effect of using an invalid
pointer value (including passing it to a deallocation function) is
undefined."
Whether "using" here refers to dereference or to lvalue to rvalue
conversion is an interesting, although a bit academic, question. But
see:
http://groups.google.com/groups?selm=hnsngr-ya023180000306000631030001%40news.pacbell.net
for an example of an architecture that uses (used?) the stricter
meaning.
jnor...@gmail.com wrote:
> Correct me if I'm wrong, but a pointer is copyable no matter what it
> points to. The container never dereference the pointer.
No, you are wrong. Copying a pointer which does not point to a
valid object and which does not point one-past-the-end of an
array invokes undefined behaviour by the standard.
Frank
> Thomas Mang wrote:
> > std::vector requires its instantiation type to be copyable. At all
> > times.
> > When you call delete p, then the pointer remaining in the vector is
> not
> > copyable any more.
>
> Correct me if I'm wrong, but a pointer is copyable no matter what it
> points to. The container never dereference the pointer.
An indeterminate pointer value, such as the value held by a pointer
which has been deleted, cannot be read or copied without invoking
undefined behaviour. Of course, most platforms do allow you to
read all pointer values without harm, but it's not required by the
C++ standard.
-- James
(or use the previously mentioned trick of swaping a zero in with the
deleted value).
>* megi...@gmail.com (Mike Gibson)
>| Now, after using this vector, I need to destroy all the memory I
>| have allocated
>
>If you'd put smart pointers in the vector, the pointers would handle
>the deletion for you when they're destroyed by the vectors DTOR.
smart pointers, brrr
>Other than that, why not
>
> while (!myvector.empty()) {
> delete myvector.back();
> myvector.pop_back();
> }
>
>Has the advantage of not leaving dangling pointers in the vector (ok,
>the vector will cease to exist right after this, but...)
I prefer your solution to the 'for_each' solutions presented. Taking
into accout what others have said about pointer invalidation after
delete a generic destroy function template would look like this:
template <typename VectorT>
void tear_down (VectorT& myVector) throw() {
typename VectorT::value_type p = 0;
while (!myVector.empty()) {
p = myVector.back();
myVector.pop_back();
try {
delete p;
} catch (...) {}
}
}
Best wishes,
Roland Pibinger
Efficiency. Your version passes for_each a pointer-to-function, and it
conceptually dispatches by indirecting through that pointer - a run-time
operation. Knowing you have a pointer-to-function isn't enough to tell you
which function you are pointing to.
The earlier versions make better use of the type-system; they dispatch at
compile-time. Knowing you have a pointer to a delete_ptr<T> tells you
exactly which delete_ptr<T>::operator() you will call, because there is
only one and it is not virtual. So it is easier for the compiler to inline
and make efficient.
-- Dave Harris, Nottingham, UK
Catalin Pitis wrote:
> "Didier Trosset" <didier-do...@acqiris.com> wrote in message
> news:41779417$0$834$626a...@news.free.fr...
>
>>Mike Gibson wrote:
>>
>>--- 8< ---
>>
>>
>>>template <class T>
>>>struct delete_ptr : std::unary_function<T*, void>
>>>{
>>> void operator() (T* p) const
>>> {
>>> delete p;
>>> }
>>>};
>>>
>>>std::for_each(my_vector.begin(), my_vector.end(), delete_ptr<A>());
>>>
>>
>>I've been adviced a nicer version:
>>
>> struct delete_ptr
>> {
>> template <class P>
>> void operator() (P p)
>> {
>> delete p;
>> }
>> };
>>
>> std::for_each(my_vector.begin(), my_vector.end(), delete_ptr());
>>
>>
>>Note that you don't have to specify the type of the objects you want to
>>delete anymore.
>>
>
>
> Why not a shorter version:
> template <typename T>
> void delete_pointer( T* p)
> {
> delete p;
> }
>
> for_each( my_vector.begin(), my_vector.end(), delete_pointer);
>
>
Because it does not compile! (g++ (GCC) 3.3.4 (Debian 1:3.3.4-13))
Didier
> An indeterminate pointer value, such as the value held by a pointer
> which has been deleted, cannot be read or copied without invoking
> undefined behaviour. Of course, most platforms do allow you to
> read all pointer values without harm, but it's not required by the
> C++ standard.
Could you please name the platforms that don't allow that?
--
Maxim Yegorushkin
> > --- 8< ---
> >> template <class T>
> >> struct delete_ptr : std::unary_function<T*, void>
> >> {
> >> void operator() (T* p) const
> >> {
> >> delete p;
> >> }
> >> };
> >> std::for_each(my_vector.begin(), my_vector.end(), delete_ptr<A>());
> > I've been adviced a nicer version:
> > struct delete_ptr
> > {
> > template <class P>
> > void operator() (P p)
(The function should be const, I think.)
> > {
> > delete p;
> > }
> > };
> > std::for_each(my_vector.begin(), my_vector.end(), delete_ptr());
> > Note that you don't have to specify the type of the objects you want
> > to delete anymore.
> Why not a shorter version:
> template <typename T>
> void delete_pointer( T* p)
> {
> delete p;
> }
> for_each( my_vector.begin(), my_vector.end(), delete_pointer);
Maybe because it doesn't compile. You've defined an infinity of
delete_pointer functions. The compiler has to choose one to
instantiate the template. How?
--
James Kanze GABI Software http://www.gabi-soft.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
for(size_t i=0; i<myvector.size(); ++i) delete myvector[i];
myvector.clear();
which of the following statements are true about the above statements
a) Legal code;
b) Strictly speaking, has undefined behavior, but would be expected to work
on all reasonable implementations,
c) Undefined behavior and would be expected to NOT work on some or all
reasonable implementations.
"Ralf Fassel" <ral...@gmx.de> wrote in message
news:ygahdoo...@ozelot.akutech-local.de...
>* megi...@gmail.com (Mike Gibson)
> | Now, after using this vector, I need to destroy all the memory I
> | have allocated
>
> If you'd put smart pointers in the vector, the pointers would handle
> the deletion for you when they're destroyed by the vectors DTOR.
>
> Other than that, why not
>
> while (!myvector.empty()) {
> delete myvector.back();
> myvector.pop_back();
> }
>
> Has the advantage of not leaving dangling pointers in the vector (ok,
> the vector will cease to exist right after this, but...)
I didn't try to compile it. I just sketched the idea. At a first look, I
don't see why it doesn't compile...
Catalin
Ok then, how can I earn efficiency (other way than defining a functor)?
Catalin
> > An indeterminate pointer value, such as the value held by a pointer
> > which has been deleted, cannot be read or copied without invoking
> > undefined behaviour. Of course, most platforms do allow you to
> > read all pointer values without harm, but it's not required by the
> > C++ standard.
> Could you please name the platforms that don't allow that?
Technically speaking, none of the platforms I know of allow it. That is
to say that the standard says it is undefined behavior, and the platform
does not explicitly define it, so it remains undefined behavior.
It happens to work, at least most of the time, on my Sun Sparcs, and on
my Linux machine. They don't allow it, but they don't do anything to
forbid it either. I've used compilers for Intel 80386 where it didn't
work, however. (Well, one, anyway.) And I've heard rumors that it
sometimes causes problems on other platforms.
--
James Kanze GABI Software http://www.gabi-soft.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
Which one?
And how exactly it "did not work"?
Can you give code example and describe what went wrong with it?
I am just curious. It seems to me that it is not trivial to create an i386
implementation where accessing a valid memory location (albeit
with "invalid" content) "does not to work".
Ivan
Because delete_pointer doesn't have a type, and none can be deduced
for it in that context. delete_ptr(), however, does have a very
definite type, and avoids that problem. There are definite limits
to the magic of type deduction in general and in C++ in particular.
-- James
If member functions aren't exception safe, they surely the object
isn't safe to use anyway, is it?
Or do you mean that member functions must not throw? If so, how does
a member function throwing cause a leak here? std::vector isn't going
to call member functions anyway (an algorithm might if passed one as a
functor, is that what you mean?)
Perhaps you are referring to the danger of using remove algorithms
since they wouldn't delete the objects pointed to?
for_each takes a functor, in the sense of something which can
be called with function notation. Given the Copyable requirement,
that means a function pointer or a functor object of class type.
There are no other options; using a functor object is the way to
avoid the probable overhead of using a function pointer.
-- James
I think it's (b).
How about
for(size_t i=0; i<myvector.size(); ++i)
{
delete myvector[i];
myvector[i] = 0;
}
myvector.clear();
Ivan
> > Catalin Pitis wrote:
[...]
> > > Why not a shorter version:
> > > template <typename T>
> > > void delete_pointer( T* p)
> > > {
> > > delete p;
> > > }
> > > for_each( my_vector.begin(), my_vector.end(), delete_pointer);
> > Because it does not compile! (g++ (GCC) 3.3.4 (Debian 1:3.3.4-13))
> I didn't try to compile it. I just sketched the idea. At a first look,
> I don't see why it doesn't compile...
Since an earlier answer of mine apparently got lost: At first look, I
don't see how it could compile. Thanks to the power of templates, you
have an infinite number of overloaded delete_pointer functions. You
pass the address of one to a template function which can take any type
for that particular parameter (again, an infinite set of overloaded
functions -- the only constraints for the compiler on for_each is that
the first two arguments have the same type). How on earth is the
compiler to resolve overload, when you have an infinite number of
overloaded functions which could be exact matches?
Using your solution, you have to write:
for_each( v.begin(), v.end(), (void (*)( Type* )delete_pointer ) ;
Note that the solution with the member template works because the type
of the object passed to for_each is NOT a template. So the compiler
knows it, and can instantiate for_each. Only within for_each does it
need the template member function, and then, it has a parameter with a
type which permits unambiguous instantiation.
--
James Kanze GABI Software http://www.gabi-soft.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
> >> > An indeterminate pointer value, such as the value held by a
> >> > pointer which has been deleted, cannot be read or copied
> >> > without invoking undefined behaviour. Of course, most platforms
> >> > do allow you to read all pointer values without harm, but it's
> >> > not required by the C++ standard.
> >> Could you please name the platforms that don't allow that?
> > I've used compilers for Intel 80386 where it didn't
> > work, however. (Well, one, anyway.)
> Which one?
The Intel C compiler. Ca 1990. When compiling with 48 bit pointers, with
an underlying real-time kernal.
> And how exactly it "did not work"?
Something along the lines of:
free( p ) ;
if ( p == NULL ) // ...
for example, would cause a system crash.
Basically, malloc and free when directly to the RT kernel. Which always
allocated segments, and invalidated them on free. The compiler
generated code which systematically loaded the pointer into a
segment/register pair when the pointer was accessed, and loading an
invalid segment into a segment register caused a hardware trap, which
crashed the system.
> Can you give code example and describe what went wrong with it?
> I am just curious. It seems to me that it is not trivial to create an
> i386 implementation where accessing a valid memory location (albeit
> with "invalid" content) "does not to work".
Once you've freed the pointer, it no longer points to a valid memory
location.
--
James Kanze GABI Software http://www.gabi-soft.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
Ivan Krivyakov wrote:
> How about
>
> for(size_t i=0; i<myvector.size(); ++i)
> {
> delete myvector[i];
> myvector[i] = 0;
> }
>
> myvector.clear();
There is still a short period where the vector is left with an
invalid pointer. Thus you have to remove the pointer from the
vector *before* you call delete on the pointer.
Frank
no, but we're dealing with a but different notion here. The exception-safety
of std::vector assumes
that you store objects that are themselves execption-safe. Heap-allocated
objects are not themselves exception-safe, hence the problem.
| Or do you mean that member functions must not throw?
no.
|If so, how does
| a member function throwing cause a leak here? std::vector isn't going
| to call member functions anyway (an algorithm might if passed one as a
| functor, is that what you mean?)
no.
| Perhaps you are referring to the danger of using remove algorithms
| since they wouldn't delete the objects pointed to?
no.
Consider the approach out-lined by Mike:
template< class T >
struct my_vector : std::vector<T*>
{
~my_vector() { /* delete pointers */ }
};
my_vector<U> vec;
vec.push_back( new U );
is the last line safe? No.
A correct implementation would look like
template< class T >
my_vector<T>::push_back( T* r )
{
std::auto_ptr<T> ptr( r );
base->push_back( r );
ptr.release();
}
And similar or worse problems will appear for many members of vector's
interface. Hence my advice to use my
library which (probably as the only lib yet made) do all this stuff correctly,
br
Thorsten