Billy ONeal
Billy ONeal

Reputation: 106539

In either C or C++, should I check pointer parameters against NULL/nullptr?

This question was inspired by this answer.

I've always been of the philosophy that the callee is never responsible when the caller does something stupid, like passing of invalid parameters. I have arrived at this conclusion for several reasons, but perhaps the most important one comes from this article:

Everything not defined is undefined.

If a function doesn't say in it's docs that it's valid to pass nullptr, then you damn well better not be passing nullptr to that function. I don't think it's the responsibility of the callee to deal with such things.

However, I know there are going to be some who disagree with me. I'm curious whether or not I should be checking for these things, and why.

Upvotes: 43

Views: 10798

Answers (19)

Coriiander Drewitt
Coriiander Drewitt

Reputation: 93

He who performas invalid operations on invalid or nonexisting data, only deserves his system-state to become invalid.

I consider it complete nonsense that functions which expect input should check for NULL. Or whatever other value for that matter. The sole job of a function is to do a task based on its input or scope-state, nothing else. If you have no valid input, or no input at all, then don't even call the function. Besides, a NULL-check doesn't detect the other millions and millions of possible invalid values. You know on forehand you would be passing NULL, so why would you still pass it, waste valuable cycles on yet another function call with parameter passing, an in-function comparison of some pointer, and then check the function output again for success or not. Sure, I might have done so when I was 6 years old back in 1982, but those days have long since gone.

There is ofcourse the argument to be made for public API's. Like some DLL offering idiot-proof checking. You know, those arguments: "If the user supplies NULL you don't want your application to crash." What a non-argument. It is the user which passes bogus data in the first place; it's an explicit choice and nothing else than that. If one feels that is quality, well... I prefer solid logic and performance over such things. Besides, a programmer is supposed to know what he's doing. If he's operating on invalid data for the particular scope, then he has no business calling himself a programmer. I see no reason to downgrade the performance, increase power consumption, while increasing binary size which in turn affects instruction caching and branch-prediction, of my products in order to support such users.

Upvotes: 1

Jaywalker
Jaywalker

Reputation: 3119

Overhead of development time + runtime performance has a trade-off with the robustness of the API you are designing.

If the API you are publishing has to run inside the process of the calling routine, you SHOULD NOT check for NULL or invalid arguments. In this scenario, if you crash, the client program crashes and the developer using your API should mend his ways.

However, if you are providing a runtime/ framework which will run the client program inside it (e.g., you are writing a virtual machine or a middleware which can host the code or an operating system), you should definitely check of the correctness of the arguments passed. You don't want your program to be blamed for the mistakes of a plugin.

Upvotes: 0

Jonathan Wood
Jonathan Wood

Reputation: 67195

One side effect of that approach is that when your library crashes in response to being passed an invalid argument, you will tend to get the blame.

There is no better example of this than the Windows operating system. Initially, Microsoft's approach was to eliminate many tests for bogus arguments. The result was an operating system that was more efficient.

However, the reality is that invalid arguments are passed all time. From programmers that aren't up to snuff, or just using values returned by other functions there weren't expected to be NULL. Now, Windows performs more validation and is less efficient as a result.

If you want to allow your routines to crash, then don't test for invalid parameters.

Upvotes: 0

AShelly
AShelly

Reputation: 35540

My philosophy is: Your users should be allowed to make mistakes, your programming team should not.

What this means is that the only place you should check for invalid parameters including NULL, is in the top-level user interface. Everywhere the user can provide input to your code, you should check for errors, and handle them as gracefully as possible.

Everywhere else, you should use ASSERTS to ensure the programmers are using the functions correctly.

If you are writing an API, then only the top-level functions should catch and handle bad input. It is pointless to keep checking for a NULL pointer three or four levels deep into your call stack.

Upvotes: 5

Loki Astari
Loki Astari

Reputation: 264391

If you don't want a NULL then don't make the parameter a pointer.
By using a reference you guarantee that the object will not be NULL.

Upvotes: 1

JCCyC
JCCyC

Reputation: 16622

One thing you have to consider is what happens if some caller DOES misuse your API. In the case of passing NULL pointers, the result is an obvious crash, so it's OK not to check. Any misuse will be readily apparent to the calling code's developer.

The infamous glibc debacle is another thing entirely. The misuse resulted in actually useful behavior for the caller, and the API stayed that way for decades. Then they changed it.

In this case, the API developers' should have checked values with an assert or some similar mechanism. But you can't go back in time to correct an error. The wailing and gnashing of teeth were inevitable. Read all about it here.

Upvotes: 1

Stuart Golodetz
Stuart Golodetz

Reputation: 20616

There is a distinction between what I would call legal and moral responsibility in this case. As an analogy, suppose you see a man with poor eyesight walking towards a cliff edge, blithely unaware of its existence. As far as your legal responsibility goes, it would in general not be possible to successfully prosecute you if you fail to warn him and he carries on walking, falls off the cliff and dies. On the other hand, you had an opportunity to warn him -- you were in a position to save his life, and you deliberately chose not to do so. The average person tends to regard such behaviour with contempt, judging that you had a moral responsibility to do the right thing.

How does this apply to the question at hand? Simple -- the callee is not "legally" responsible for the actions of the caller, stupid or otherwise, such as passing in invalid input. On the other hand, when things go belly up and it is observed that a simple check within your function could have saved the caller from his own stupidity, you will end up sharing some of the moral responsibility for what has happened.

There is of course a trade-off going on here, dependent on how much the check actually costs you. Returning to the analogy, suppose that you found out that the same stranger was inching slowly towards a cliff on the other side of the world, and that by spending your life savings to fly there and warn him, you could save him. Very few people would judge you entirely harshly if, in this particular situation, you neglected to do so (let's assume that the telephone has not been invented, for the purposes of this analogy). In coding terms, however, if the check is as simple as checking for NULL, you are remiss if you fail to do so, even if the "real" blame in the situation lies with the caller.

Upvotes: -1

greyfade
greyfade

Reputation: 25647

In my opinion, it's the callee's responsibility to enforce its contract.

If the callee shouldn't accept NULL, then it should assert that.

Otherwise, the callee should be well behaved when it's handed a NULL. That is, either it should functionally be a no-op, return an error code, or allocate its own memory, depending on the contract that you specified for it. It should do whatever seems to be the most sensible from the caller's perspective.

As the user of the API, I want to be able to continue using it without having the program crash; I want to be able to recover at the least or shut down gracefully at worst.

Upvotes: 0

Stephane Rolland
Stephane Rolland

Reputation: 39906

I am pro defensive programming.

Unless you can profile that these nullptr checkings happen in a bottleneck of your application... (in such cases it is conceivable one should not do those pointers value tests at those points)

but all in all comparing an int with 0 is really cheap an operation. I think it is a shame to let potential crash bugs instead of consuming so little CPU.

so: Test your pointers against NULL!

Upvotes: 3

R.. GitHub STOP HELPING ICE
R.. GitHub STOP HELPING ICE

Reputation: 215259

If you're going to check for NULL pointer arguments where you have not entered into a contract to accept and interpret them, do it with an assert, not a conditional error return. This way the bugs in the caller will be immediately detected and can be fixed, and it makes it easy to disable the overhead in production builds. I question the value of the assert except as documentation however; a segfault from dereferencing the NULL pointer is just as effective for debugging.

If you return an error code to a caller which has already proven itself buggy, the most likely result is that the caller will ignore the error, and bad things will happen much later down the line when the original cause of the error has become difficult or impossible to track down. Why is it reasonable to assume the caller will ignore the error you return? Because the caller already ignored the error return of malloc or fopen or some other library-specific allocation function which returned NULL to indicate an error!

Upvotes: 50

Nemanja Trifunovic
Nemanja Trifunovic

Reputation: 24561

While in general I don't see the value in detecting NULL (why NULL and not some other invalid address?) for a public API I'd probably still do it simply because many C and C++ programmers expect such behavior.

Upvotes: 16

Cheers and hth. - Alf
Cheers and hth. - Alf

Reputation: 145269

For C++, if your function doesn't accept nullpointer, then use a reference argument. In general.

There are some exceptions. For example, many people, including myself, think it's better with pointer argument when the actual argument will most naturally be a pointer, especially when the function stores away of a copy of the pointer. Even when the function doesn't support nullpointer argument.

How much to defend against invalid argument depends, including that it depends on subjective opinion and gut-feeling.

Cheers & hth.,

Upvotes: 1

Steve Townsend
Steve Townsend

Reputation: 54148

Defense in Depth principle says yes. If this is an external API then totally essential. Otherwise, at least an assert to assist in debugging misuse of your API.

You can document the contract until you are blue in the face, but you cannot in callee code prevent ill-advised or malicious misuse of your function. The decision you have to make is what's the likely cost of misuse.

Upvotes: 9

Karl Knechtel
Karl Knechtel

Reputation: 61519

In C++, if you don't want to accept NULL pointers, then don't take the chance: accept a reference instead.

Upvotes: 31

Victor Nicollet
Victor Nicollet

Reputation: 24577

The answer is going to be different for C and C++.

C++ has references. The only difference between passing a pointer and passing a reference is that the pointer can be null. So, if the writer of the called function expects a pointer argument and forgets to do something sane when it's null, he's silly, crazy or writing C-with-classes.

Either way, this is not a matter of who wears the responsibility hat. In order to write good software, the two programmers must co-operate, and it is the responsibility of all programmers to 1° avoid special cases that would require this kind of decision and 2° when that fails, write code that blows up in a non-ambiguous and documented way in order to help with debugging.

So, sure, you can point and laugh at the caller because he messed up and "everything not defined is undefined" and had to spend one hour debugging a simple null pointer bug, but your team wasted some precious time on that.

Upvotes: 4

wkl
wkl

Reputation: 79921

I lean heavily on the side of 'don't trust your user's input to not blow up your system' and in defensive programming in general. Since I have made APIs in a past life, I have seen users of the libraries pass in null pointers and then application crashes result.

If it is truly an internal library and I'm the only person (or only a select few) have the ability to use it, then I might ease up on null pointer checks as long as everyone agrees to abide by general contracts. I can't trust the user base at large to adhere to that.

Upvotes: 4

Michael K
Michael K

Reputation: 3347

I think that you should strive to write code that is robust for every conceivable situation. Passing a NULL pointer to a function is very common; therefore, your code should check for it and deal with it, usually by returning an error value. Library functions should NOT crash an application.

Upvotes: 1

Android Eve
Android Eve

Reputation: 14974

In my view, it's not a question of responsibility. It's a question of robustness.

Unless I have full control on the caller and I must optimize for even the minute speed improvement, I always check for NULL.

Upvotes: 7

Andrey
Andrey

Reputation: 60065

I don't think it's the responsibility of the callee to deal with such things

If it doesn't take this responsibility it might create bad results, like dereferencing NULL pointers. Problem is that it always implicitly takes this responsibility. That's why i prefer graceful handling.

Upvotes: 0

Related Questions