Is it safe to delete a NULL pointer?

Question

Is it safe to delete a NULL pointer?

And is it a good coding style?

Solution 1

delete performs the check anyway, so checking it on your side adds overhead and looks uglier. A very good practice is setting the pointer to NULL after delete (helps avoiding double deletion and other similar memory corruption problems).

I'd also love if delete by default was setting the parameter to NULL like in

#define my_delete(x) {delete x; x = NULL;}

(I know about R and L values, but wouldn't it be nice?)

Solution 2

From the C++0x draft Standard.

$5.3.5/2 - "[...]In either alternative, the value of the operand of delete may be a null pointer value.[...'"

Of course, no one would ever do 'delete' of a pointer with NULL value, but it is safe to do. Ideally one should not have code that does deletion of a NULL pointer. But it is sometimes useful when deletion of pointers (e.g. in a container) happens in a loop. Since delete of a NULL pointer value is safe, one can really write the deletion logic without explicit checks for NULL operand to delete.

As an aside, C Standard $7.20.3.2 also says that 'free' on a NULL pointer does no action.

The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs.

Solution 3

Yes it is safe.

There's no harm in deleting a null pointer; it often reduces the number of tests at the tail of a function if the unallocated pointers are initialized to zero and then simply deleted.

---

Since the previous sentence has caused confusion, an example — which isn't exception safe — of what is being described:

void somefunc(void)
{
    SomeType *pst = 0;
    AnotherType *pat = 0;

    …
    pst = new SomeType;
    …
    if (…)
    {
        pat = new AnotherType[10];
        …
    }
    if (…)
    {
        …code using pat sometimes…
    }

    delete[] pat;
    delete pst;
}

There are all sorts of nits that can be picked with the sample code, but the concept is (I hope) clear. The pointer variables are initialized to zero so that the delete operations at the end of the function do not need to test whether they're non-null in the source code; the library code performs that check anyway.

Solution 4

Deleting a null pointer has no effect. It's not good coding style necessarily because it's not needed, but it's not bad either.

If you are searching for good coding practices consider using smart pointers instead so then you don't need to delete at all.

Solution 5

To complement ruslik's answer, in C++14 you can use this construction:

delete std::exchange(heapObject, nullptr);

View more solutions

Comments

[sth]

Note that there still can be several other pointers pointing to the same object even if you set one to NULL on deletion.

[ruslik]

@sth sure, but it's still better than nothing. In most cases there is only one pointer.

[josesuero]

In most cases in my code, the pointer goes out of scope once it's been deleted. Much safer than merely setting it to NULL.

[Tony Delroy]

the time people want to delete a NULL pointer is when they're not sure if it contains NULL... if they knew it was NULL then they wouldn't be considering delete and hence asking ;-).

[josesuero]

what about them? They're typically smart pointers, or they're members of my own ad hoc RAII class, which, like I said, goes out of scope when the pointer is deleted -- or they point to an object whose lifetime extends past that of the object containing the pointer.

[MSalters]

@ruslik: Class members go out of scope when the containing object goes out of scope. Hence, in the dtor, there's no point in setting them to zero. Now, in an assignment operator, it may be necessary to delete a pointer to a subobject, but only if you already have a new suboject ! It's not exception-safe to delete the old object first - you could end up with no subobject at all. Therefore, you wouldn't set the pointer to NULL; you'd set it to the new object.

[Brian R. Bondy]

@Tony: My point was only that it will have no effect, and the presence of such code which deletes a pointer which sometimes contains NULL is not necessarily bad.

[fredoverflow]

@ruslik: raw pointer members are an extremely bad idea in modern C++.

[Konrad Rudolph]

This is illegal, and I don’t believe it.

[Mysticial]

You should be able to delete any null pointer. So if it's breaking for you, then you probably have a bug in the code that shows it.

[Damon]

A very god practice is not setting the pointer to NULL after delete. Setting a pointer to NULL after deleting it masquerades memory allocation errors, which is a very bad thing. A program that is correct does not delete a pointer twice, and a program that does delete a pointer twice should crash.

[paulm]

IMO redundant checks certainly are bad, for performance, readability and maintainability.

[Alice]

@Damon A program that deletes a pointer set to an object, and then deletes a pointer set to NULL, is correct. That's what the standard says.

[Damon]

@Alice: It is irrelevant what the standard says in that respect. The standard defined deleting a null pointer being valid for some absurd reason 30 years ago, so it is legal (most likely a C legacy). But deleting the same pointer twice (even after changing its bit pattern) is still a serious program error. Not by the wording of the standard, but by program logic and ownership. As is deleting a null pointer, since the null pointer corresponds to no object, so nothing could possibly be deleted. A program must exactly know if an object is valid and who owns it, and when it can be deleted.

[Damon]

@Alice: Worded differently, this unlucky rule of the standard allows for programs that behave erroneously (hopefully I spelled that right) to execute as-if correct. When in fact, the validity and ownership of objects isn't clear at all (you could say it's guesswork). It only works by accident because the problem is silently ignored. A better approach would be to crash early (or throw, if you will), which forces the developer to make sure the program's logic is correct. Setting a pointer to (T*)1 instead of nullptr will do that, it will crash hard if you try to delete it again.

[Alice]

@Damon The correctness of a program is dictated by if a programs behavior is correct with respect to a specification. Part of that spec is user related (does the code do what I want) and part is standards related (does it follow the rules of the language). In both cases, a program which frees a NULL is correct; it follows the rules of the language, and causes no errors. It does not work "by accident"; this was discussed, and included into the standard, not merely as legacy but because it is useful and removes checks for NULL in areas where they are pointless. It works as C++ was designed.

[Alice]

@Damon You are imposing your own beliefs onto the area of code correctness: pointers represent a weak reference option type, with NULL indicating no value. Deleting on an option type should be, by type theory, defined for the optional part as well; otherwise it's not optional. So, you must be able to safely delete NULL; otherwise, you break the type contract, and therefore have no way to prove correctness. Ownership is irrelevant: pointers are weak for a reason, use shared_ptr if that's your use case. A program need not know ownership exactly; it need only not undermine it's constraints.

[Damon]

@Alice: C++ was standardized 8 years after being invented by a single person. The standard committee had to assure above all one thing: Making sure that the vast majority of code that relied on this mis-feature did not break. Insofar there was no real "decision". Your interpretation of the standard is of course correct, but it is pedantic. A program that does not know when an object is valid is incorrect. Resource ownership is not gambling or looking in a crystal ball or something you leave to chance otherwise. It is something that is unambiguous and well-defined in a well-written program.

[Alice]

@Damon First, that isn't true; the standard made several changes that caused a vast majority of code to break. See the history of the STL for details. Second, program correctness does not exist in a vacuum; you must prove a program correct against a specification. Well definedness has a specification: does the program do the same thing each time it runs with the same values? Finally, ambiguity means two equally valid results come from the same values. Correctness implies well defined, which implies ambiguity. None of these in any way deal with knowing whether an object is valid.

[Alice]

@Damon Deleting a NULL is not ambiguous; it does the same thing every time. It is well defined; the specification exists and says what it does, and it does the same thing every time. And it is correct; both the IO of the program match spec, and it matches type theories spec on optional types. Furthermore, object validity has no impact on any of these; weak pointers may point to valid or invalid objects, but are not ambiguous, ill defined, or incorrect. And without extra work, it is impossible to prove if they are valid or not; it is unknowable. Is any C++ program with raw pointers incorrect?

[Damon]

@Alice: You don't get it. If you have a pointer, this pointer is either uninitialized/invalid, or it points to an object (without owning it), or it owns an object. No matter what, this is something that must be exactly and unambiguously known at all times. There is exactly one instance, the owning pointer, which is allowed (and required) to delete the object at a well-defined time. If that is not the case, your program logic is wrong. Deleting NULL being legal and setting invalid pointers to that value hides this problem. It makes programs that don't get ownership right "well behaving".

[Alice]

@Damon What you said is just false. Suppose I have a raw pointer to some object, as does some framework (this occurs often in QT, WT, etc). The framework deletes that object, possibly in response to code I have no control over. Now I have a pointer to an invalid object, but I don't know it's invalid (and in fact, can't know, as much of the code I am linking is beyond my control). Furthermore, no notion of an owning pointer exists; all pointers can delete the object they point to, and none are required to; it's perfectly valid to allow the heap to clean up trivial objects on program close.

[Alice]

@Damon The flaw here is that you have a draconian view of ownership that neither the standard, type theory, or logic in general require. Each of your "requirements" can be overturned with no loss of correctness; multiple pointers may delete, no pointers need own, the time of delete need not be well defined, and weak pointers need not know the object they point to is invalid. But none of that has to do with deleting NULL being legal or setting pointers to NULL to hide double free's; in fact doing so enhances correctness, because it means programs do not exhibit incorrect or undefined behavior.

[Damon]

@Alice: I wish you good luck for the future as several threads of exectuion are becoming (or in fact are already) the norm, not the exception. Ideas like "any pointer can delete" or "ownership and time of deletion needs not be well-defined" will help you a great deal writing robust programs and will earn you a lot of love among your co-workers.

[Alice]

@Damon I'm a maintainer of several lock free structures, and attempting to publish a paper on a novel lock free trie. One of the main characteristics of a lock free structure is that a thread may need to help another thread, rather than progress in its own right. This often necessitates several pointers each capable of deleting. This is, in fact, provably true for some algorithms on hardware with only CAS, not DCAS (most commercial hardware). As well, the time when deletes can occur in such scenarios are often not well defined, in the sense they will not occur at the same time each execution.

[Alice]

@Damon However, despite these abrogations of your draconian ownership rules, lock free structures are provably more robust than lock based ones. And yes, my co-workers do indeed love me for the enhanced execution profile these structures provide and the rigorous thread safety they maintain, which allow easier to reason about code (great for maintenance). However, none of this nor your implied personal attack have to do with any definition of correctness, validity, or ownership. What you propose is a good rule of thumb, but it is not a universal law nor is it enshrined in the standard.

[sp2danny]

§5.3.2 In the second alternative (delete array), the value of the operand of delete may be a null pointer value or a pointer value that resulted from a previous array new-expression.

[user207421]

I had to read that a few times to make sense of it. You must mean initializing them to zero at the top of the method, or during it, not at the tail, surely? Otherwise you would just remove both the zeroing and the delete.

[Jonathan Leffler]

@EJP: A not wholly implausible outline of a function might be: void func(void) { X *x1 = 0; Y *y1 = 0; … x1 = new[10] X; … y1 = new[10] Y; … delete[] y1; delete[] x1; }. I've not shown any block structure or jumps, but the delete[] operations at the end are safe because of the initializations at the start. If something jumped to the end after x1 was allocated and before y1 was allocated and there was no initialization of y1, then there'd be undefined behaviour — and while the code could test for nullness (of x1 and y1) before the deletions, there is no need to do so.

[Jim Balter]

"I know about R and L values" -- But apparently you don't know about references. <template T> void my_delete(T*& p) { delete p; p = NULL; }

[Jim Balter]

@Alice "weak pointers need not know the object they point to is invalid" -- they do if they can be deleted; in that case they need a reference count to avoid multiple deallocation. " But none of that has to do with deleting NULL being legal or setting pointers to NULL to hide double free's" -- More likely a pointer is set to NULL after delete to return it to a "reset" state. This is a perfectly valid semantics, and was in use long before owned pointers ... which have a memory cost that Damon seems unaware of. There are still machines with finite memory where such things matter. :-)

[Alice]

JimBalter That is an excellent point that @Damon also seems unaware of: constraints have costs, and C deliberately avoids many constraints explicitly because of the costs involved. Deleting a NULL is entirely justifiable if it makes an otherwise slow concept of ownership faster with no loss of safety (which is demonstrably true in many cases). Why use a slower mechanism for safety if it provides no benefits?

[dewtell]

If reuse of stale pointers was always guaranteed to lead to an immediate crash, I might agree with @Damon's recommendation. But memory gets reused, in both predictable (stack) and unpredictable (heap) locations. And non-null pointers can get passed around and copied many times before they get dereferenced, producing hidden aliasing and Heisenbugs of various flavors far from the site of the original problem. Having had to debug a few of these bugs over the course of my career, I am a firm believer in resetting stale pointers to NULL as soon as possible, as a basic sanitation move.

[TOMKA]

@Alice: I know it's been some time, but I'm interesting in reading your paper on lock free tries, and also wondering if you had a github or similar for your other lock free structures?

[codetaku]

I would really like this answer for its citations if it didn't intentionally introduce inefficiency in non-optimized code. As the accepted answer states, deletion of a null pointer is a no-op. Therefore, checking if a pointer is null before deleting it is completely extraneous.

[Pockets]

I wonder where the "move to chat" suggestion went in this comment thread...

[Curious]

@Damon Totally agree. Setting a pointer to nullptr can can also have important performance implications. For example, if you have a tree data structure and rely on recursion for deletion and decide to set a pointer to nullptr after calling delete, the stack frames cannot be reused as in tail recursion. It can many times be enough to cause a stack overflow if you set a pointer to nullptr after deleting.

[WhiZTiM]

NOTE: Using a MACRO will cause double evaluation if an expression is used. example: my_delete(Order3PizzasAndBurnTheBakery())

[Opux]

@KonradRudolph What's illegal?

[Konrad Rudolph]

@Opux The behaviour of VS2010 alleged in the answer. As explained in other comments, it's safe to delete[] NULL.

[Opux]

@KonradRudolph Maybe, but VS is known for its quirks, so it's at least plausible.

[Konrad Rudolph]

@Opux That's why I wrote “I don't believe it” rather than “that's wrong”. But I still don't, and it would be a pretty outrageous, stupid violation of the standard. VC++ is actually generally pretty good at following the standard's restrictions, and the places where it violates them make sense historically.

[Winter]

@paulm OP is certainly not talking about that kind of bad, more of Seg Fault / UB kind of bad.

[Przemek]

@WhiZTiM this example is invalid. Macro argument need to be an lvalue, and function call isn't, so it won't compile. But you've got the point. For function int** foo(void){static int *arr = new int(4); return &arr;} calling my_delete(*foo()) will allocate, delete, then allocate again and finally nullify arr (no pointers to allocated memory - memory leak). On the other hand my_delete(x) {auto _tmp = &(x); .. will fail if x is direct function call (not an lvalue). I wonder why such error-prone code is in accepted answer.

[underscore_d]

@Przemek "Macro argument need to be an lvalue" ...What? That is trivially false. Macros don't know anything about value categories, objects, or any language features; they are just brainless substitution devices. A macro argument containing the textual representation of a function call or any other expression will be substituted into the body of the macro however many times that argument occurs, and so the expression will be evaluated - and the function called - that many times, regardless of what the function call returns or what value category that has.

[Przemek]

@underscore_d you've taken my words out of context, perhaps my wording wasn't the best one. Look at definition of 'my_delete' - after macro expansion there is assignment to macro argument. You are correct about macro preprocessor being simply textual mechanism that will just copy-paste the definition, but resultant code will be incorrect (at compile time).

[Qwertiy]

Just checked in VS2010 - they both work fine with null: i.stack.imgur.com/Ug327.png

[Admin]

It's nice to know it is safe. But one might nevertheless use a "if (mypointer)" clause because it signals to someone reading the code that the pointer in question is one that might not have been set, and it signals that this is not a bug or an awkwardness but part of the design (I admit a comment could also be used).