Are there any good reasons (except "macros are evil", maybe) NOT to use the following macros ?
#define DELETE( ptr ) \
if (ptr != NULL) \
{ \
delete ptr; \
ptr = NULL; \
}
#define DELETE_TABLE( ptr ) \
if (ptr != NULL) \
{ \
delete[] ptr; \
ptr = NULL; \
}
Personally I prefer the following
template< class T > void SafeDelete( T*& pVal )
{
delete pVal;
pVal = NULL;
}
template< class T > void SafeDeleteArray( T*& pVal )
{
delete[] pVal;
pVal = NULL;
}
They compile down to EXACTLY the same code in the end.
There may be some odd way you can break the #define system but, personally (And this is probably going to get me groaned ;) I don't think its much of a problem.
Because it doesn't actually solve many problems.
In practice, most dangling pointer access problems come from the fact that another pointer to the same object exists elsewhere in the program and is later used to access the object that has been deleted.
Zeroing out one of an unknown number of pointer copies might help a bit, but usually this is a pointer that is either about to go out of scope, or set to point to a new object in any case.
From a design point of view, manually calling delete
or delete[]
should be relatively rare. Using objects by value instead of dynamically allocated objects where appropriatem using std::vector
instead of dynamically allocated arrays and wrapping the ownership of objects that have to be dynamically allocated in an appropriate smart pointer (e.g. auto_ptr
, scoped_ptr
or shared_ptr
) to manage their lifetime are all design approaches that make replacing delete
and delete[]
with a "safer" macro a comparatively low benefit approach.
Because it is OK to delete a NULL(0)
pointer. The is no need to check if the pointer actually is NULL(0)
or not. If you want to set the pointer to NULL, after deleting, then you can overload the delete
operator globally with out using macros.
It seems that I was wrong about the second point:
If you want to set the pointer to
NULL, after deleting, then you can
overload the delete
operator
globally
The thing is that if you overload the global new
and delete
, you could have something like this:
void* operator new(size_t size)
{
void* ptr = malloc(size);
if(ptr != 0)
{
return ptr;
}
throw std::bad_alloc("Sorry, the allocation didn't go well!");
}
void operator delete(void* p)
{
free(p);
p = 0;
}
Now, if you set p = 0;
in the overloaded delete
, you are actually setting the local
one, but not the original p
. Basically, we are getting a copy of the pointer in the overloaded delete
.
Sorry, it was on the top of my head, I gave it a second thought now. Anyway, I would write template inline function to do the thing instead of writing EVIL MACROS :)
Because DELETE is already defined in winnt.h :
#define DELETE (0x00010000L)
Your macro fails for several reasons:
- It is a macro. It doesn't respect scoping rules or a number of other language features, making it easy to use incorrectly.
- It can cause compile-errors:
DELETE (getPtr());
won't compile, because you can't set the function call to null. Or if the pointer is const, your macro will also fail.
- It achieves nothing.
delete NULL
is allowed by the standard.
Finally, as grimner said, you're trying to solve a problem that shouldn't exist in the first place. Why are you manually calling delete at all?` Don't you use the standard library containers? Smart pointers? Stack allocation? RAII?
As Stroustrup has said before, the only way to avoid memory leaks is to avoid having to call delete.
Use boost::shared_ptr<> instead.
http://www.boost.org/doc/libs/1_39_0/libs/smart_ptr/shared_ptr.htm
The MACRO here provides some of the functionality you are probably looking for.
Yes, you should never call delete directly. Use shared_ptr,scoped_ptr,unique_ptr or whatever smart pointer you have in your project.
- It doesn't give you much benefit. Deleting a null pointer is harmless, so the only benefit is setting the pointer to NULL after the delete. If a developer can remember to call your macro rather than delete, she can also remember to null out the pointer, so you're not really protecting yourself from a careless developer. The only benefits is that this happens in two lines rather than one.
- It's potentially confusing. delete is a standard part of the language. Your macro or templated function is not. So a new developer will need to look up that macro definition to understand what your code is doing.
In my judgement, the benefit does not outweigh the cost.