NULL Checking and Defensive Programming
-
I just received a crazy lecture from one of my superiors when I tried to introduce NULL pointer checks in the code. Apparently, he says, "Pointers should never be NULL" and "Checking pointers for NULL throughout the code eats too many cpu cycles and seriously degrades performance", "It's better just to crash and provide a stack dump." Am I on crack?
I think most agree that once you declare a pointer than the next logical step should be to point it to something. Preferably that something is a value other than null. However, there are perfectly legitimate cases for assigning a null value to a pointer , for example the implementation of a linked list where the last element points to nothing (null). These situations should be by design. If you follow the practice of assigning values to pointers as soon as you declare them than you should minimize the risk of dereferencing a null pointer. However, like anything else there is no guarantee. Thus I recommend some type of centralized error handling scheme that would gracefully handle this exception as opposed to Null pointer checks prior to using the pointer.
-
I just received a crazy lecture from one of my superiors when I tried to introduce NULL pointer checks in the code. Apparently, he says, "Pointers should never be NULL" and "Checking pointers for NULL throughout the code eats too many cpu cycles and seriously degrades performance", "It's better just to crash and provide a stack dump." Am I on crack?
You have good start with your superior¡ He is right about "Checking pointers for NULL throughout the code eats too many CPU cycles and seriously degrades performance". And, of course, pointer can be NULL. Most people agree: to check pointers for NULL on every parameter in public or protected member of the class, and private and internal members shall not waste CPU to check for something that by logic will never be null. If you not sure about something you wrote, insert
Debug.Assert(C#),assert(Java), orASSERT(MS C/C++). They will be excluded from production code and it will "... just to crash and provide a stack dump" during debugging/testing. Every code piece has to be executed during unit tests. And if you have something likeif(obj == null) return false;andobjis never null according code coverage, then something wrong: either create test to cover that case (which will be impossible when it’s hidden inside private members), or just remove to not spend one CPU cycle :-). Check also http://channel9.msdn.com/shows/Going+Deep/Expert-to-Expert-Contract-Oriented-Programming-and-Spec/[^]. -
I just received a crazy lecture from one of my superiors when I tried to introduce NULL pointer checks in the code. Apparently, he says, "Pointers should never be NULL" and "Checking pointers for NULL throughout the code eats too many cpu cycles and seriously degrades performance", "It's better just to crash and provide a stack dump." Am I on crack?
You could use assert's as defensive programming and leave the production code without the checks.
-
I just received a crazy lecture from one of my superiors when I tried to introduce NULL pointer checks in the code. Apparently, he says, "Pointers should never be NULL" and "Checking pointers for NULL throughout the code eats too many cpu cycles and seriously degrades performance", "It's better just to crash and provide a stack dump." Am I on crack?
Your not on crack, I also follow the same defensive programming philosophy. Checking pointers for NULL does not harm performance at all and anyone who tells you that is mentally deficient. Instructions executed on modern processors are measured in MIPS[^] which is to say millions of instructions per second. Current processors are capable of executing billions of instructions each second. Of course checking for NULL pointers is a philosophy however its one that I follow. I always check for NULL pointers before using them and when I am done using my pointer I assign it a NULL value. Its two simple rules that I follow and guarantees that I never have a NULL pointer exception in my code. In my opinion a program should *never* crash to the desktop. I believe it is the engineers duty to detect the NULL pointer exception or other unhandled exceptions and prompt the user with a dialog stating that a critical error has occured and allow the user to attempt a graceful shutdown of the application. A combination of NULL pointer checks, variable validation and a global structured exception handler[^] gives applications stability and reliability. Not all software is created equal, I want mine to be in the top percentage. Only arrogant programmers do not follow these types of rules, they seem to think their code will never crash. Eventually they are wrong, usually when some poor guy has been working for 6 hours and forgot to save his work. Best Wishes, -David Delaune
-
I just received a crazy lecture from one of my superiors when I tried to introduce NULL pointer checks in the code. Apparently, he says, "Pointers should never be NULL" and "Checking pointers for NULL throughout the code eats too many cpu cycles and seriously degrades performance", "It's better just to crash and provide a stack dump." Am I on crack?
Performance aside, checking for NULL will give you a false sense of security. A bad pointer usually has a non-NULL value anyway. Ie.:
void my_function(MyType* object)
{
delete object;
object = NULL; // of course, this makes no sense either...
}int main()
{
MyType* object = new MyType;
my_function(object);// here, object is bad, but not NULL
if (object) // NULL check is worthless
object->do_something(); // BOOOOM!!!
}modified on Saturday, December 20, 2008 6:55 PM
-
I just received a crazy lecture from one of my superiors when I tried to introduce NULL pointer checks in the code. Apparently, he says, "Pointers should never be NULL" and "Checking pointers for NULL throughout the code eats too many cpu cycles and seriously degrades performance", "It's better just to crash and provide a stack dump." Am I on crack?
-
Performance aside, checking for NULL will give you a false sense of security. A bad pointer usually has a non-NULL value anyway. Ie.:
void my_function(MyType* object)
{
delete object;
object = NULL; // of course, this makes no sense either...
}int main()
{
MyType* object = new MyType;
my_function(object);// here, object is bad, but not NULL
if (object) // NULL check is worthless
object->do_something(); // BOOOOM!!!
}modified on Saturday, December 20, 2008 6:55 PM
Protect against what you can anyway.
-
I just received a crazy lecture from one of my superiors when I tried to introduce NULL pointer checks in the code. Apparently, he says, "Pointers should never be NULL" and "Checking pointers for NULL throughout the code eats too many cpu cycles and seriously degrades performance", "It's better just to crash and provide a stack dump." Am I on crack?
Finding null pointer risks is not easy at all, even with carefull re reading. I can help finding then (nearly!!) all in C#, VB6 and java. For this risk and lots of others, have a look at http://d.cr.free.fr/indexen.html
-
Protect against what you can anyway.
PIEBALDconsult wrote:
Protect against what you can anyway.
Sorry, not good enough :) The only sane way to avoid this kind of problems is to keep the object alive within the scope it is used and have no pointers pointing to it out of that scope. Checking for NULL is helpful only in cases NULL is a valid parameter to a function (meaning - ignore this parameter). As a safety measure, it is completelly worthless - they are billions (on 32-bit systems) possible invalid values for a pointer - why checking for 0 only?
-
PIEBALDconsult wrote:
Protect against what you can anyway.
Sorry, not good enough :) The only sane way to avoid this kind of problems is to keep the object alive within the scope it is used and have no pointers pointing to it out of that scope. Checking for NULL is helpful only in cases NULL is a valid parameter to a function (meaning - ignore this parameter). As a safety measure, it is completelly worthless - they are billions (on 32-bit systems) possible invalid values for a pointer - why checking for 0 only?
Because I can, and a null pointer is generally more likely than an invalid pointer. Again I'll invoke the similarity to a condom; it may not protect against everything, and it may reduce performance somewhat, but it does protect against some specific things. Do you wear a seat belt? There are some people [weasel words] who argue against them, saying, "what if I drive off a bridge into a lake and drown because I can't undo the seat belt?" I wear a seat belt; a crash is far more likely than falling into a lake. Locking your car or house is more of a hindrance to you than to a serious thief; do you do it anyway? I do. If I write a method that takes several pointers, I can check each one and tell the caller exactly which parameter(s) were null, rather than simply blowing up and making the post-mortem team guess what happened.
-
PIEBALDconsult wrote:
Protect against what you can anyway.
Sorry, not good enough :) The only sane way to avoid this kind of problems is to keep the object alive within the scope it is used and have no pointers pointing to it out of that scope. Checking for NULL is helpful only in cases NULL is a valid parameter to a function (meaning - ignore this parameter). As a safety measure, it is completelly worthless - they are billions (on 32-bit systems) possible invalid values for a pointer - why checking for 0 only?
Nemanja Trifunovic wrote:
why checking for 0 only?
Heh, in some of the software I have worked on, there are humans whos life could be in danger if my code crashes as the software controls the vehicle/vessel motion and trajectory. In addition to checking for NULL... in some places in that code you might find me checking my pointers with:
ASSERT(0 == ((DWORD_PTR)pAddress & 3));I wonder if that qualifies as a WTF :-D If I am working with function pointers in some mission critical code I might check the function pointer with something like:
PDWORD pAddress = (PDWORD)&SomeFunction; DWORD_PTR pInstructions = *pAddress; MEMORY_BASIC_INFORMATION mbi; if(VirtualQueryEx(GetCurrentProcess(),(LPVOID)pInstructions,&mbi,sizeof(MEMORY_BASIC_INFORMATION))) { if(mbi.Protect & PAGE_EXECUTE_READ && mbi.State & MEM_COMMIT && mbi.AllocationProtect & PAGE_EXECUTE_WRITECOPY && mbi.Type & MEM_IMAGE) { (*this.*SomeFunction)(val); } }If humans were riding in vehicles and your code was driving perhaps you would also begin checking for NULL pointers. To make matters worse, just imagine if the vehicles/vessels are worth several hundred million dollars. :(( Now let me ask you a rhetorical question. Would you trust the team who wrote Visual Studio 2008 to drive that vehicle? pFunction->BOOM() :~ Best Wishes, -David Delaune
-
Because I can, and a null pointer is generally more likely than an invalid pointer. Again I'll invoke the similarity to a condom; it may not protect against everything, and it may reduce performance somewhat, but it does protect against some specific things. Do you wear a seat belt? There are some people [weasel words] who argue against them, saying, "what if I drive off a bridge into a lake and drown because I can't undo the seat belt?" I wear a seat belt; a crash is far more likely than falling into a lake. Locking your car or house is more of a hindrance to you than to a serious thief; do you do it anyway? I do. If I write a method that takes several pointers, I can check each one and tell the caller exactly which parameter(s) were null, rather than simply blowing up and making the post-mortem team guess what happened.
PIEBALDconsult wrote:
Because I can,
You can also check for 0x00000001 which is also an invalid value on most systems. Then, you can also check for 0x00000002, 0x00000003, and all other values you know are invalid. Again, why is 0 specific?
PIEBALDconsult wrote:
and a null pointer is generally more likely than an invalid pointer
I completelly dissagree here. A pointer will have a value NULL only if you explicitly set it to NULL - an unitialized pointer is not going to be NULL, and neither a "dangling" pointer.
PIEBALDconsult wrote:
Again I'll invoke the similarity to a condom
Sorry, there is no similarity at all. Condom protects from some but not all dangers. Checking for NULL protects against nothing.
PIEBALDconsult wrote:
If I write a method that takes several pointers, I can check each one and tell the caller exactly which parameter(s) were null, rather than simply blowing up and making the post-mortem team guess what happened.
Your check makes sense only if your function takes input pointers that can legally be zero, and then ignore them. As an error detection, it is worthless. If a caller passes a NULL pointer to a function, it means he set it to be NULL; detecting a NULL here makes sense only if the function is documented to allow NULL as an option.
-
Your not on crack, I also follow the same defensive programming philosophy. Checking pointers for NULL does not harm performance at all and anyone who tells you that is mentally deficient. Instructions executed on modern processors are measured in MIPS[^] which is to say millions of instructions per second. Current processors are capable of executing billions of instructions each second. Of course checking for NULL pointers is a philosophy however its one that I follow. I always check for NULL pointers before using them and when I am done using my pointer I assign it a NULL value. Its two simple rules that I follow and guarantees that I never have a NULL pointer exception in my code. In my opinion a program should *never* crash to the desktop. I believe it is the engineers duty to detect the NULL pointer exception or other unhandled exceptions and prompt the user with a dialog stating that a critical error has occured and allow the user to attempt a graceful shutdown of the application. A combination of NULL pointer checks, variable validation and a global structured exception handler[^] gives applications stability and reliability. Not all software is created equal, I want mine to be in the top percentage. Only arrogant programmers do not follow these types of rules, they seem to think their code will never crash. Eventually they are wrong, usually when some poor guy has been working for 6 hours and forgot to save his work. Best Wishes, -David Delaune
Randor wrote:
Of course checking for NULL pointers is a philosophy however its one that I follow. I always check for NULL pointers before using them and when I am done using my pointer I assign it a NULL value. Its two simple rules that I follow and guarantees that I never have a NULL pointer exception in my code.
There is no such thing as a "NULL pointer exception" - there is "access violation" and if you follow your rules you may still very easily run into it, as demonstrated in the code snippet here[^].
-
Nemanja Trifunovic wrote:
why checking for 0 only?
Heh, in some of the software I have worked on, there are humans whos life could be in danger if my code crashes as the software controls the vehicle/vessel motion and trajectory. In addition to checking for NULL... in some places in that code you might find me checking my pointers with:
ASSERT(0 == ((DWORD_PTR)pAddress & 3));I wonder if that qualifies as a WTF :-D If I am working with function pointers in some mission critical code I might check the function pointer with something like:
PDWORD pAddress = (PDWORD)&SomeFunction; DWORD_PTR pInstructions = *pAddress; MEMORY_BASIC_INFORMATION mbi; if(VirtualQueryEx(GetCurrentProcess(),(LPVOID)pInstructions,&mbi,sizeof(MEMORY_BASIC_INFORMATION))) { if(mbi.Protect & PAGE_EXECUTE_READ && mbi.State & MEM_COMMIT && mbi.AllocationProtect & PAGE_EXECUTE_WRITECOPY && mbi.Type & MEM_IMAGE) { (*this.*SomeFunction)(val); } }If humans were riding in vehicles and your code was driving perhaps you would also begin checking for NULL pointers. To make matters worse, just imagine if the vehicles/vessels are worth several hundred million dollars. :(( Now let me ask you a rhetorical question. Would you trust the team who wrote Visual Studio 2008 to drive that vehicle? pFunction->BOOM() :~ Best Wishes, -David Delaune
I am sure you have some point you want to prove here, but it escapes me :) If you are saying that checking pointer for NULL is going to make your programs more robusts, I think I already demonstrated you are wrong. You can check for NULL all you want and still have an access violation.
-
PIEBALDconsult wrote:
Because I can,
You can also check for 0x00000001 which is also an invalid value on most systems. Then, you can also check for 0x00000002, 0x00000003, and all other values you know are invalid. Again, why is 0 specific?
PIEBALDconsult wrote:
and a null pointer is generally more likely than an invalid pointer
I completelly dissagree here. A pointer will have a value NULL only if you explicitly set it to NULL - an unitialized pointer is not going to be NULL, and neither a "dangling" pointer.
PIEBALDconsult wrote:
Again I'll invoke the similarity to a condom
Sorry, there is no similarity at all. Condom protects from some but not all dangers. Checking for NULL protects against nothing.
PIEBALDconsult wrote:
If I write a method that takes several pointers, I can check each one and tell the caller exactly which parameter(s) were null, rather than simply blowing up and making the post-mortem team guess what happened.
Your check makes sense only if your function takes input pointers that can legally be zero, and then ignore them. As an error detection, it is worthless. If a caller passes a NULL pointer to a function, it means he set it to be NULL; detecting a NULL here makes sense only if the function is documented to allow NULL as an option.
Nemanja Trifunovic wrote:
NULL only if you explicitly set it to NULL - an unitialized pointer is not going to be NULL
C99 and C# initialize pointers (references) to NULL. Retraction: OK, I misread the C99 spec; I saw, "-- if it has pointer type, it is initialized to a null pointer;" without reading the lead-in, which indicates that that's only true for static, not automatic, storage. :sigh: If you don't assign NULL to pointer variables when freed then you're on your own.
-
I am sure you have some point you want to prove here, but it escapes me :) If you are saying that checking pointer for NULL is going to make your programs more robusts, I think I already demonstrated you are wrong. You can check for NULL all you want and still have an access violation.
Nemanja Trifunovic wrote:
I think I already demonstrated
No, while your point of view is valid, it carries little weight with us, as ours seem to with you. A program that checks for NULL pointers is (likely) more robust; we're not saying it will never crash, we're just saying it won't crash on something as simple to test as a NULL pointer, or if it does, it should at least give a clearer indication of what went wrong. Corrie ten Boom[^] didn't save all the Jews in Holland, but she did what she could. Doing nothing because you can't do everything is not a way to go through life.
-
Nemanja Trifunovic wrote:
NULL only if you explicitly set it to NULL - an unitialized pointer is not going to be NULL
C99 and C# initialize pointers (references) to NULL. Retraction: OK, I misread the C99 spec; I saw, "-- if it has pointer type, it is initialized to a null pointer;" without reading the lead-in, which indicates that that's only true for static, not automatic, storage. :sigh: If you don't assign NULL to pointer variables when freed then you're on your own.
PIEBALDconsult wrote:
If you don't assign NULL to pointer variables when freed then you're on your own.
So what if I do? That would not zero any other pointer that point to the deleted object. In my snippet here[^] I did assign the pointer to NULL after deleting it just to point that it doesn't help at all.
-
Performance aside, checking for NULL will give you a false sense of security. A bad pointer usually has a non-NULL value anyway. Ie.:
void my_function(MyType* object)
{
delete object;
object = NULL; // of course, this makes no sense either...
}int main()
{
MyType* object = new MyType;
my_function(object);// here, object is bad, but not NULL
if (object) // NULL check is worthless
object->do_something(); // BOOOOM!!!
}modified on Saturday, December 20, 2008 6:55 PM
But in that example you didn't set the pointer to NULL and you know it. A called method should not free something that was passed in, or if it's expected to, you'll need double indirection. Find a better example, that one's a coding horror on its own.
-
Randor wrote:
Of course checking for NULL pointers is a philosophy however its one that I follow. I always check for NULL pointers before using them and when I am done using my pointer I assign it a NULL value. Its two simple rules that I follow and guarantees that I never have a NULL pointer exception in my code.
There is no such thing as a "NULL pointer exception" - there is "access violation" and if you follow your rules you may still very easily run into it, as demonstrated in the code snippet here[^].
Nemanja Trifunovic wrote:
There is no such thing as a "NULL pointer exception" - there is "access violation"
Nemanja, Come on man I know your not really that pedantic about verbiage. Call it whatever you want, "Access Violation" is simply an error title. Now your just trying to be argumentative. I'll call it segmentation fault[^] from now on just to get under your skin.
Nemanja Trifunovic wrote:
if you follow your rules you may still very easily run into it
First off let me say that I have respect for both you and your philosophy. I can only hope to recieve the same. But unfortunately I have a partially opposing view. There are many techniques to making software more robust. Assigning pointers to NULL and checking for NULL is only one of them and many companies practice it as can be seen in both Microsoft DDK and Platform SDK samples. Logically from a mathematical statistics standpoint any technique which reduces the chance of accessing an invalid memory address would be beneficial to the software. Magic Number[^] assignment/checking is used by all major software vendors.. Apple, Microsoft and IBM etcetera. Recognize any of these? 0xCDCDCDCD 0xBAADF00D 0xCCCCCCCC Initializing a pointer to NULL to denote an invalid memory address is the same magic number technique. It assists the programmer with validating pointers and most certainly assists with making software more robust. I am not interested in arguing the point anymore. I fully expect you to reject my assertions and you are within your rights to do so. Let us agree that we disagree and leave it at that. Best Wishes, -David Delaune
-
PIEBALDconsult wrote:
Because I can,
You can also check for 0x00000001 which is also an invalid value on most systems. Then, you can also check for 0x00000002, 0x00000003, and all other values you know are invalid. Again, why is 0 specific?
PIEBALDconsult wrote:
and a null pointer is generally more likely than an invalid pointer
I completelly dissagree here. A pointer will have a value NULL only if you explicitly set it to NULL - an unitialized pointer is not going to be NULL, and neither a "dangling" pointer.
PIEBALDconsult wrote:
Again I'll invoke the similarity to a condom
Sorry, there is no similarity at all. Condom protects from some but not all dangers. Checking for NULL protects against nothing.
PIEBALDconsult wrote:
If I write a method that takes several pointers, I can check each one and tell the caller exactly which parameter(s) were null, rather than simply blowing up and making the post-mortem team guess what happened.
Your check makes sense only if your function takes input pointers that can legally be zero, and then ignore them. As an error detection, it is worthless. If a caller passes a NULL pointer to a function, it means he set it to be NULL; detecting a NULL here makes sense only if the function is documented to allow NULL as an option.
To chime in with the biggest noob problem, at least in the VB.NET forum, what's the default for a newly created, but uninitialized pointer?? I haven't written any C++ code in quite a while, but I believe it was 0.
A guide to posting questions on CodeProject[^]
Dave Kreskowiak Microsoft MVP Visual Developer - Visual Basic
2006, 2007, 2008