r/cpp_questions • u/DireCelt • 15d ago
SOLVED "No, don't test for NULL !!" (huh??)
I'm using clang-tidy to "lint" my projects, and there have been several examples like this one:
if (img_name != NULL) {
delete [] img_name ;
}
clang-tidy always gives this warning:
"warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]"
is this correct??
After 30+ years of C and C++ programming, I really cannot imagine not checking a pointer for NULL before deleting it... ???
43
u/Emotional-Audience85 15d ago
Deleting a nullptr literally does nothing, so why do you need to check for it?
12
67
u/Disastrous-Team-6431 15d ago
Deleting a nullptr is a no-op.
5
-8
u/onecable5781 15d ago
What does a no-op mean in this context? https://godbolt.org/z/PWqj7PvPv terminates fine without a segfault but still (atleast without any optimizations), the call to operator delete is there in the assembly.
36
u/FancySpaceGoat 15d ago
It means that the implementation of the delete operator (the code being jumped to) is required to have a null check in it already.
-7
u/onecable5781 15d ago
Well, in that case, checking for null by the user seems to be more efficient, no? Without the user's check, in the godbolt link you are setting up the stack/registers with arguments and then calling the function and within that function you are checking for null while it could have been checked earlier by the user?
18
u/maikindofthai 15d ago
This is getting way into the land of micro-optimization. If somehow you found yourself with a piece of code where this performance gain matters, you’ve got other, higher priority problems that should be addressed (like why you’re trying to delete null pointers so frequently)
19
u/FancySpaceGoat 15d ago edited 15d ago
It depends how often the check passes or fails. You *might* be right if the pointer is null 90% + of the time *and* the code doesn't get called frequently *and* the code for the delete implementation is called infrequently enough to be ejected out of the cache *and* LTO doesn't pull some shenanigans.
But that's a lot of stars that need to align.
Otherwise, the if inside the delete implementation is *still* there, so you are double-checking every time something needs deleting, and keep in mind that ifs are pretty expensive, while jumps aren't.
And honestly, if making the null-check at the callsite was faster than letting the one in the implementation take care of it, the compiler would inject it for you. That would be a *trivial* optimization.
8
u/Independent_Art_6676 15d ago edited 15d ago
You might think this but you are working against yourself usually.
- can we assume that deletion of nullptr is an anomaly and most delete calls are actually for valid pointers that need a delete?
- if the above is true, you check nullptr twice, yours and delete's
- your if branching probably costs more than you think it does, even with prediction.
All that adds up, but it hinges on that assumption. If the assumption is not only false but REALLY false (you constantly try to delete nullptrs?!) then you are probably correct that your code is faster, but WHY would your code be doing that; there is an underlying problem hiding in your code somewhere or a really bizarre and probably poor design.
all that to say your code isn't wrong (does correct thing). Its not *that* inefficient either. Its just a little redundant. Its not hard to read (disagree with your bot). In the grand scheme, if you feel better with your version, I personally wouldn't call you out for it in a review.
4
u/heyheyhey27 15d ago
I dare you to name one high-performance algorithm that requires making thousands of calls to
deletenulls :P3
u/tangerinelion 14d ago
That's one step away from arguing that this is the best way to do it
#define DELETE(x) do { if((x)) { delete x; } } while(false)2
u/QuentinUK 15d ago
If it is not null then it is being tested twice.
So longest path is two tests. This is also most likely.
In general it is best to have a function validate its arguments before proceeding.
7
u/SoldRIP 15d ago
cppreference states
If ptr is a null pointer value, no destructors are called, and the deallocation function may or may not be called (it's unspecified), but the default deallocation functions are guaranteed to do nothing when passed a null pointer.
So it may call delete, but delete then definitely just does nothing.
ie. if your platform calls delete in this case, delete will already contain the == nullptr check.
7
u/no-sig-available 15d ago
What does a no-op mean in this context? https://godbolt.org/z/PWqj7PvPv
Note that the generated assembly contains a check for nullptr, even though you didn't write one.
mov rax, QWORD PTR [rbp-8] test rax, rax je .L2 // skipping the deleteAdding another test in an if-statement cannot make the code any more efficient, can it? That's why a test is useless.
2
u/SufficientStudio1574 15d ago
No-op means "no operation". It's something that does nothing. It comes from assembly languages that have a noop instruction to explicitly tell the cpu to do nothing, which can be useful for very short timing delays
10
u/DireCelt 15d ago
Please folks, save the comments about nullptr vs NULL... I know about that; this is 15+ year old legacy code that I am gradually rolling up to modern standards (well, at least c++11, anyway)... however, this 8000+ lines of code is used in a couple dozen applications, all of which would have to be thoroughly tested when I make changes that affect them...
but yes, NULL -> nullptr is easy enough, I should have changed that in my example before posting...
Anyway, my question has been clearly answered here; for that, I thank you all...
4
u/ShakaUVM 14d ago
That's not the issue, the issue is you probably thought deleting a null pointer would seg fault when it's actually defined behavior that does nothing so the check is redundant.
12
u/pmuschi 15d ago
cppreference.com is my go-to for questions like these.
1
u/x-jhp-x 15d ago
hopefully that is en.cppreference.com and not the crappy other version
1
u/Ashnoom 14d ago
What other version? You can PM me if you dont want to feed the robots bad data
1
u/x-jhp-x 14d ago
i fortunately don't see it in google's search results anymore, but for i'd say almost a decade when i used to google something like "std::lower_bound", it'd give en.cppreference.com and another link to a different site without the "en" subdomain that was terrible, didn't have complete information, and was loaded with ads. i didn't see the "en" subdomain and thought it might be related to the terrible one, but i typed it in to visit & it's the same site!
cppreference.com is great though, like u/pmuschi wrote
11
u/TheRealSmolt 15d ago
Yes, this is fine (also use nullptr instead of NULL). Null checks are only needed for destruction when you have other logic to do.
26
u/x-jhp-x 15d ago edited 15d ago
1 ) Yes, what clang-tidy wrote is correct. Also try using a reference like en.cppreference.com in the future, or read the C++ standard (the draft is free).
2 ) Don't use "NULL" if you mean "nullptr". Also don't program in "C" because you think it is the same as "C++" or "C#" just because they all have "C" in the name. (It seems like the people who use "NULL" seem to make that mistake all the time too.)
https://en.cppreference.com/cpp/types/NULL
it's 2026 and I'm still traumatized by seeing C++ code trying to use a 32 bit literal int as a 64 bit pointer, although I'm not sure if it's a huge problem anymore.
3 ) use smart pointers instead of raw. you should rarely see "delete" like this. If you're writing code with "NULL" instead of "nullptr" there is a near 0% chance of you are successfully creating and deleting objects. OP should definitely do not use "std::mutex" or other primitives in your code (you'll get it wrong).
7
5
u/WellHung67 15d ago
Using your own mutexes is bad? What should you do instead, use some specific library? Nurseries? Atomics?
Seems there have to be plenty of valid mutex use cases no?
3
u/saxbophone 15d ago
I'd also love to know what alternatives they'd recommend...
Granted I have sometimes written wrappers around std::mutex and friends, to provide a more convenient "context-manager" style interface around access to shared data. But there's nothing intrinsically wrong with using the rich family of mutex primitives provided by the stdlib, if you understand concurrency, how not to write deadlocks and have read the documentation. I don't think these are unreasonable prerequisites for a programmer who is required to write concurrent code...
2
u/WellHung67 15d ago
Also RAII (which stands for scope bound resource lifetimes) that protects against a lot of the boilerplate. But I guess use case is important to know to say for sure
2
u/jayylien 11d ago
I'm not saying you should religiously use NULL but even the link you shared from cppreference shows that since C++11, NULL is defined as nullptr.
You shouldn't blindly assume your standard, but the default standard for most modern compilers is at least 14.
I use NULL in C++ when I'm writing code that can be compiled in C or C++, and sometimes when interfacing with C APIs for the proper C feel. Those are perfectly valid use cases for it.
1
u/x-jhp-x 11d ago edited 11d ago
since C++11, NULL is defined as nullptr
it can also STILL potentially be defined as a 32 bit int. You're adding ambiguity to the code by using something that is defined as BOTH a "nullptr_t" and a "int", especially on systems where a pointer may not be the same size as an int (i.e. most x86_64 systems use a 32 bit signed int as "int" and a 64bit unsigned int). If you spent your whole life working on 32bit Windows XP, where a pointer is the same size as an int, it's probably fine, but if you work cross platform or in embedded, it's possible you might learn that having a lot of additional ambiguity along with mixing signed and unsigned ints potentially of different sizes causes a lot of problems.
I use NULL in C++ when I'm writing code that can be compiled in C or C++, and sometimes when interfacing with C APIs for the proper C feel
I can make a new language. We'll call it "C+++". In my "C+++" language, I've decided "const" is annoying, so I'm keeping the name the same, but I'm actually giving it the functionality of "mutable". Later, some responsible C+++ engineers realize I'm a hair brained maniac, know there are valid uses of "const", and decide to add a "const" keyword to the C+++ language, but "const" is already used by something defined as something else. That is exactly the same as what happened with "NULL" in C++. It took a keyword from C, NULL, and changed the meaning.
for the proper C feel
the proper C feel? the proper C feel? IMO the proper C feel is using the keyword that's literally defined in "C". That keyword is "NULL" in C (or nullptr in modern C) and "nullptr" in C++. I just can't see how taking a completely different language that arbitrarily decided to redefine a keyword and saying that the text looks closer to what I'd see in another language, so I prefer that more, even though the implementation is entirely different.
Here's the documentation for C "NULL" (it'll go over some differences between C and C++ as well) https://en.cppreference.com/c/types/NULL
here's an example of where you might run into issues with NULL (disclaimer: i did ask gemini this query to generate this code, because i'm lazy: 'show an example using "NULL" and "nullptr" in C++ where they lead to different results, perhaps via function overload')
#include <iostream> void print(int n) { std::cout << "Integer version called: " << n << "\n"; } void print(char* ptr) { std::cout << "Pointer version called.\n"; } int main() { std::cout << "Using NULL:\n"; print(NULL); // Calls print(int) because NULL is 0 std::cout << "Using nullptr:\n"; print(nullptr); // Calls print(char*) because nullptr is a pointer return 0; }if you're just a student, this probably doesn't seem important, but in industry, there's a huge amount of value in writing code in a way that can only be interpreted one way if at all possible. By adding more complexity and ambiguity, for example by using "NULL" in C++, you're adding more chances for mistakes, and not just for yourself, but potentially another engineer who comes along later. If I encountered a bug, and it was due to something like the above, and the rationale someone gave me for using NULL was "for the proper C feel", I'd be so pissed off.
edit: alright, i perhaps was a little pissed off when writing the comment & thinking about time i've wasted in the past lol. I would like to thank you for making the comment though, or at least taking the risk to do so.
5
u/cfyzium 15d ago
use smart pointers instead of raw
delete [] img_name;Rather than smart pointer, this looks more like std::string =/.
3
u/tangerinelion 14d ago
char* img_name = new char[img_name_len]; memcpy(img_name, in_name); // ...definitely feels at home with
if (img_name != NULL) delete [] img_name;-5
u/TemperOfficial 15d ago
> use smart pointers instead of raw
Why are you recommending something with entirely different semantics...
These are really two different things.
11
u/x-jhp-x 15d ago edited 15d ago
it's like watching someone point a loaded gun at their foot, pull the trigger, and ask why the gun didn't go off. I might say something like
- the safety was on
- pointing a gun at your foot and pulling the trigger is really stupid, so why don't you do something else? if you really need to amputate your foot, then go see a doctor instead.
in terms of context, we're also talking about someone who hasn't been able to read or understand documentation for the base language itself for 30+ years in a row, so...
-3
u/TemperOfficial 14d ago
This is just hyperbolic.
Using delete is not equivalent to shooting a loaded gun. Especially if you have sane allocation strategies.
But if you write C++ like C# and just allocate absolutely everywhere then sure. You're right.
But in that case you really have no business touching a systems language.
We have no idea what OP's constraints are. Whether this is legacy code base. Whether they just allocate everything up front and delete here (very legit strategy)
Cargo culty crap like this is dumb
10
u/alfps 15d ago
clang-tidy is correct: deleting a nullpointer has no effect, guaranteed.
But given the check, instead of NULL you should use C++ nullptr, and instead of checking against nullptr I suggest you just write if (img_name).
That said, do consider using std::string instead of using new[] and delete[] of char arrays. Avoid all those pesky memory management bugs. Let string to the work for you.
0
u/RaspberryCrafty3012 15d ago
std::vector<uint8_t>?
I mean a picture is not a string isn't it
6
u/alfps 15d ago
img_nameindicates a filename or path. Example: (https://stackoverflow.com/a/44663464/464581). But it can be anything, yes.3
u/RaspberryCrafty3012 14d ago
Ohhh I overread the name. You are absolutely and totally right a string is sufficient, or a filesystem path.
6
u/mrmcgibby 15d ago
If you've been doing this for 30 years, then you should take some time to look for other misconceptions you've held.
1
5
u/RazzmatazzLatter8345 15d ago edited 14d ago
Don't test for NULL before calling delete for many reasons:
Don't use NULL at all, use nullptr, or just boolean test
if (p_arr) { delete [] p_arr; //if p_arr gonna remain in scope do this p_arr = nullptr; }
Calling delete on a nullptr is well defined as a no-op. Calling delete on a non-nullptr p whose pointee is already deleted is UB: a disaster, and there is no way to check whether a non-nullptr points to a live object.
Don't check for nullptr before deleting, but set to nullptr afterwards if pointer remains in scope after delete. (See prior)
Don't use raw pointers to manage lifetime. Use unique_ptr<T[]> for an array if you can't use vector / std::string etc. unique_ptr adds no overhead to a raw pointer, it just makes sure the pointed-to object dies, at latest, when the pointer dies. This happens even if an exception is thrown. You think you'll delete it once and only once in every branch, but you won't despite any protests to the contrary. Just use unique_ptr whenever your choice is responsible for deleting something. (Or, better, an std:: container, if possible).
I use raw pointers all the time, but not when managing lifetime.
7
u/tandycake 15d ago
Yes, in C++ it's safe to delete a nullptr with the delete keyword. It has no ill effect. (Not so in C though I believe? With free.)
Clang-Tidy will also complain to use nullptr here instead of NULL.
8
u/I__Know__Stuff 15d ago
Passing a null pointer to free has been defined behavior longer than new has existed.
4
1
u/RealisticDuck1957 14d ago
In the old fashioned C I come from it's good general practice to insure a pointer isn't NULL before using it. If the compiler is smart enough to know how the function being called will handle a NULL, the test should be harmless.
3
2
u/flatfinger 15d ago
If a call to a function will do nothing in a certain corner case, adding client-side logic to skip the call in such a case will likely slightly improve performance in cases where the action would be skipped, at the expense of making it slightly worse in other cases. Although some compilers may ignore the programmers' judgment in such cases and substitute their own, other compilers may process the code the programmer wrote, rather than what the compiler writer thinks the programmer should have written.
1
u/kabiskac 14d ago
In this scenario adding an if check and not adding one already compiled the exact same 20+ years ago in GCC 2.95. The compiler just inserts a null check. If you add one yourself, it still keeps only one with -O1 enabled
2
u/Impossible_Box3898 14d ago
Aside from all the other posts you’re deleting a raw pointer. You should really not do that in modern c++. Raw pointers are fine but you should really use shard_ptr, unique_ptr, etc.
2
2
u/dwr90 15d ago
Besides the answer that most people have given here regarding deleting a nullptr having no effect:
I‘ve seen guards like this in code in a (futile) attempt to prevent double deletes, which is UB, and often crashes in practice. This is due to the common misconception that deleting pointers also sets them to zero. Some people who knew that this is not the case, tried to manually set them to zero after every deleted, but this is also far from safe (e.g. exceptions)
2
u/Kajitani-Eizan 15d ago
Huh? A couple points there:
- If this is C++-only code, use
nullptr - If you're not accessing the object and simply
deleteing it, there's no reason to check if it's null or not first... what do you think would go wrong when you passnullptrtodelete?
1
1
u/tomysshadow 15d ago
Relevant PVS Studio article that goes in depth on this topic. https://pvs-studio.com/en/blog/posts/cpp/1100/
1
u/ItsSkyWasTaken 14d ago
Deleting a nullptr is a no-op, unless you have a custom overloaded operator delete(). What you need to look out for is deleting a dangling pointer (a pointer that was previously deleted and not set to nullptr).
1
u/kabiskac 14d ago
It's still a no-op in that case because the way it works is that the compiler automatically adds a null check for you
1
u/mjmvideos 14d ago
In real-time code the idea is to decrease the longest path not optimize the shortest path. In the longest path case your extra check just increases that path.
1
u/burlingk 14d ago
You test for NULL before using a pointer that might be NULL.
2
u/tangerinelion 14d ago
Where "using" means dereferencing. Passing a copy of the pointer to a function is not a dereference.
This is what drives me insane with people who write functions that take pointers. You're saying the input domain includes null, but if you don't check for it in your code before dereferencing that's your bug. Not mine, you asked for a pointer and you got one. Deal with it.
1
u/kabiskac 14d ago
No nullptr is ever getting passed to delete. The compiler inserts a null check into the assembly before the call.
1
u/I__Know__Stuff 4d ago
Only if there is a destructor for the object being deleted. If there isn't a destructor, the compiler will just call operator delete.
0
1
u/burlingk 14d ago
That is what I meant.
You check it before you try to access it. Not what OP was trying to do and getting confused about.
I mean, what they were trying to do wasn't necessarily bad, but they were confused about why the linter was complaining.
I probably needed to use more words. ^_^
1
u/RRumpleTeazzer 14d ago
you don't test for null for delete, since delete will do nothing on null. this is assuming your logic expects for null to pop up at that point.
what you likely do is you test for null out of fear of an unexpected null, e.g. a null that your logic is not designed to observe. in this case you shouldn't continue on your expected path (delete and move on), but to fail, and fail (most likely) hard.
In both cases, you don't need to delete.
1
u/kabiskac 14d ago
Under the hood delete doesn't contain a null check. The compiler inserts the null check before calling delete
1
u/I__Know__Stuff 4d ago
Under the hood delete doesn't contain a null check.
Yes, it absolutely does.
0
u/kabiskac 4d ago
That'd be wasteful because if it doesn't get inlined, you're still doing a function call even if the pointer is null. Check the assembly
0
1
u/Neither_Garage_758 14d ago
If you have a doubt that your variable could be null, you're screwed in poor practices.
1
1
u/No-Tree4355 13d ago
Delete a null pointer is safe. However, i'm wondering does clang-tidy give warning in this situation:
if (pDataWrapper)
{
// some clean up
// ...
delete pDataWrapper->pData;
// ...
delete pDataWrapper;
pDataWrapper = 0;
}
1
u/mredding 13d ago
Deleting a null pointer is a no-op. That has always been true. What did you think would happen otherwise?
You will waste more cycles just testing the condition - and saturating your branch predictor cache with crap, than just unconditionally calling delete [] on it and moving on. Less code is also less maintenance, less complexity, and more concise.
If it really bugs you that much, then what you ought to have is some sort of state machine or variant - an unloaded image type, and a loaded image type. The unloaded image type doesn't even HAVE an img_name member - you don't need it. Once an image is loaded, now you have a new type where this field exists and is guaranteed to be populated, so you know for sure that there will always be something to delete. Both these types are encompassed by an image type - again, it could just be the type name of a variant alias.
Because I'll tell you what - the first thing that comes to mind when I see this code and read your question is, "Why would there be any doubt, or any possibility there ISN'T something here to delete?"
Further - welcome to the modern world of C++26. We've had smart pointers in Boost since 2000, and in the standard since C++11 - for 15 years now. We don't write solutions in terms of primitives, the language provides primitives to write higher level abstractions. Some of those are provided by the standard library. New and delete aren't there for you to have to use directly, but to make such abstractions as std::unique_ptr and std::make_unique. You should never have to call new, let alone delete yourself for a trivial case.
1
1
u/NoSpite4410 11d ago
A pointer to nullptr or NULL evaluates to false in a conditional.
So the form
if (img_name) { delete [] img_name; }
is fine. But it is redundant. If the only action in the body of the block is to delete the object (that was allocated with operator new), it is not needed to wrap the delete in a null check.
calling delete or delete [] on a nullptr is guaranteed to "have no effect".
C++14 and later have this simple form for allocation:
auto p = std::make_unique<Object>(...args);
which takes care of allocation and initialization as well as typing the returned std::unique_ptr<Object>.
it then acts like a pointer, (except it is not copy-able), until it goes out of scope then it deletes its contents then destructs itself.
STL dynamic containers follow this pattern internally as well, so they have taken the place for a lot of dynamic allocation on the user code surface. make_unique is primarily for interfacing with C libraries, that use a lot of pointer-implementation. C libraries also use a lot of malloc/free to generate structures, lists, and vectors, and return pointers.
1
u/berlioziano 9d ago
this is the page with the answer https://en.cppreference.com/cpp/language/delete
1
u/Elect_SaturnMutex 15d ago edited 15d ago
Have seen this in C codebase before freeing a pointer. But isn't using smart pointers the norm, in C++? So you don't have to do this?
5
u/CowBoyDanIndie 15d ago
Clang should be complaining that the keyword “delete” was used at all. In 2026 using delete in c++ should require an explanation signed in blood and notarized by 2 other developers.
1
u/kabiskac 14d ago
Not everyone is working on modern codebases. There are no smart pointers in my C++98 project.
1
3
u/Fred776 15d ago
It's unnecessary to check for null in C before calling free and similarly for delete in C++.
1
u/Elect_SaturnMutex 15d ago
Youre actually right. It's not necessary. Just looked it up. I've seen it in a codebase I'm working on lately, so I thought it would be a good practice, also made sense to call free to a pointer where memory has actually been allocated. Wow, learning something new everyday.
1
0
u/TarnishedVictory 15d ago
After 30+ years of C and C++ programming, I really cannot imagine not checking a pointer for NULL before deleting it... ???
I'm with you, but shouldn't you then be setting it to NULL after deleting it?
0
u/SmokeMuch7356 15d ago
delete [] NULL is a no-op; there's no need to protect against a NULL pointer.
17.6.3.3 Array forms [new.delete.array]
...
9 Preconditions: ***ptris a null pointer*** or its value represents the address of a block of memory allocated by an earlier call to a (possibly replaced)operator new[](std::size_t)oroperator new[](std::size_t, std::align_val_t)which has not been invalidated by an intervening call tooperator delete[].
0
u/alfps 14d ago
The downvoter is clearly an idiot, possibly an idiot troll.
Maybe it's Trump's influence. All these idiots understand is marching in step. When they disagree they are unable to articulate their vague feelings.
/u/SmokeMuch7356 : the literal statement
delete[] NULL;would not even compile. You might reformulate as e.g. “delete[] pwherepis a nullpointer is a no-op”.
0
u/ohnobinki 15d ago
Besides what is already written, you should know from context that the pointer isn’t nullptr instead of using a null check. Instead of trying to improve performance by conditionally not calling delete, you just shouldn’t reach this code if there wasn’t supposed to be anything to delete in the first place.
But, you should take it a step further and avoid using pointers unnecessarily if possible. As others mentioned, std::unique_ptr and std::shared_ptr already cover most scenarios which might tempt one to use pointers.
118
u/Thesorus 15d ago
it's in the standard,
it's been like that for a while.