Reason why not to have a DELETE macro for C++

Question

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;             \
}

Answer 1

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.

Answer 2

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.

Answer 3

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 :)

Answer 4

Because DELETE is already defined in winnt.h :

#define DELETE (0x00010000L)

Answer 5

• delete accept a NULL pointer without problem, so the tests are superfluous.
• resetting the pointer to NULL is not always possible, so they can't be used systematically.
• the security they bring is illusory: in my experience, most dangling pointer problems comes from pointers other than the one used to delete.

Answer 6

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.

Answer 7

1. deleting a null pointer does nothing, so no need to check whether the pointer is null before deletion. Nullifying the deleted pointer might still be needed (but not in every case).

2. Macros should be avoided as much as possible, because they are hard to debug, maintain, introduce possible side effects, they are not part of namespaces, etc.

3. deleting a pointer that was not dynamically allocated with new will still be a problem...

Answer 8

1. Macros are evil. Why not use inline templated functions?
2. You can delete null ptrs.
3. In many cases you don't need to set the ptr to null - destructors for instance.

Answer 9

1. macros are evil :p Seriously, consider using inlined template functions instead
2. setting a pointer to NULL after deallocation tends to mask errors
3. encourages if (ptr != NULL) checks as a flow control mechanism. Personally, I consider this a code smell along the lines of void foo(int arg) being replaced with void foo(int arg, bool doAdvancedThings=false)
4. encourages usage of raw pointers to memory that needs to be deleted - shared_ptr and its relatives should always be used for ownership, raw pointers can be used for other access
5. encourages looking at a pointer variable after deallocation, even worse using if (ptr != NULL) instead of if (ptr)... comparing pointers is another code smell

Answer 10

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.

Answer 11

Yes, you should never call delete directly. Use shared_ptr,scoped_ptr,unique_ptr or whatever smart pointer you have in your project.

Answer 12

1. 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.
2. 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.