Is "delete p; p = NULL(nullptr);" an antipattern?

Question

Searching something on SO, I stumbled across this question and one of the comments to the most voted answer (the fifth comment to that most voted answer) suggests that delete p; p = NULL; is an antipattern. I must confess that I happen to use it quite often paring it sometimes\most of the times with the check if (NULL != p). The Man himself seems to suggest it (please see the destroy() function example) so I'm really confused to why it might be such a dreaded thing to be considered an antipattern. I use it for the following reasons:

• when I'm releasing a resource I also want to kind of invalidate it for further usage and NULL is the right tool to say a pointer is invalid
• I don't want to leave behind dangling pointers
• I want to avoid double\multiple free bugs - deleting a NULL pointer is like a nop but deleting a dangling pointer is like "shooting yourself in the foot"

Please note that I'm not asking the question in the context of the "this" pointer and let's assume we don't live in a perfect C++ world and that legacy code does exist and it has to be maintained, so please do not suggest any kind of smart pointer :).

Answer 1

Yes, I would not recommended doing it.

The reason is that the extra setting to null will only help in very limited contexts. If you are in a destructor, the pointer itself will not exist right after the destructor execution, which means that whether it is null or not does not matter.

If the pointer was copied, setting p to null will not set the rest of the pointers, and you can then hit the same problems with the extra issue that you will be expecting to find deleted pointers being null, and it won't make sense how your pointer became non-zero and yet the object is not there....

Additionally it might hide other errors, like for example if your application is trying to delete a pointer many times, by setting the pointer to null, the effect is that the second delete will be converted to a no-op, and while the application will not crash the error in the logic is still there (consider that a later edit accesses the pointer right before the delete that is not failing... how can that fail?

Answer 2

I recommend doing so.

• Obviously, it's valid in a context where NULL is a valid value of the pointer. This, of course, means if it's used in other places, it must be checked.
• Even if the pointer may, technically, not be NULL, it does help in real-world scenarios when customers send you a crash dump. If it's NULL and it's not supposed to be (and it wasn't trapped in testing with the assert() that you should do), then it's easy to see that this is the problem - you'll crash at something like mov eax, [edx+4] or something, you'll see that edx is 0, and you know what the problem is. If, on the other hand, you don't NULL the pointer, but it is deleted, then all sorts of things may happen. It may work, it may crash immediately, it may crash later, it may show weird things - at this point, anything that happens is soft-non-deterministic.
• Defensive programming is King. That includes setting a pointer to NULL extraneously, even if you think you don't have to, and doing that extra NULL check in a few places even though you technically know it isn't supposed to happen.
• Even if you have the logic error of a pointer going through delete twice, it's better to not crash and handle it safely than to crash. That may mean you do that extra check, and you'll probably want to log that, or maybe even end the program gracefully, but it's not just a crash. Customers don't like that.

Personally, I use an inline function that takes the pointer as a reference and sets it to NULL.