Solved

Why do we check nullity before DELETE operation?

Posted on 2000-03-24
16
246 Views
Last Modified: 2010-04-02
Why should we write
      
      if( pVal != NULL )
            delete pVal;      // 1

instead of

      delete pVal;            // 2

when NULL is a valid parameter to DELETE?

I think the 2nd is much clearer.
Is it a matter of performance? Would anyone reccomend the 2nd?

0
Comment
Question by:perizi
  • 5
  • 2
  • 2
  • +6
16 Comments
 

Expert Comment

by:jpjpjp
ID: 2652727
The check on the ptr is probably due to an old habit from C, where you cannot "free" a null pointer (the implementation of free doesnt check it the ptr is null before deleting it). The problem with that is NULL is in protected memory area, so the program core dumps when it tries to read it.  

In C++, you can use the #2, ie no need to check if the ptr is null, since the implementation of delete checks if the content is null before deleting it.
0
 
LVL 32

Expert Comment

by:jhance
ID: 2652735
You do not have to do this and it's possible that your program will be faster (depending on how often you check for NULL).  Checking for NULL is good practice, however.

Usually when this happens it's due to an error in your programming, however, so it's a good idea to put something in there to flag it for you and you can examine what caused it.

0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2652766
#1 is unnecessary, will take extra time for the check and is not good practice at all.

The argument that it was a hangover from c is not really valid, because free works fine with a NULL pointer as well.

I think code like #1 it is more based on misunderstanding of the language and paranoia.

So #2 is the better solution.

Of course, if you 'know' that the pointer should not be null, then you ay want to put in an ASSERT .. but the check for null as shown is a waste of time.

However, what IS good practice is to clear pointers to NULL after deleting (or, in C, after calling free)

delete pval;
pval = NULL;

this IS good programming practice as it doesn't leave a dangling pointer.

PS: don't forget to use delete[] for allocated arrays !!

0
 
LVL 30

Expert Comment

by:Zoppo
ID: 2653041
I agree with jhance's 'Checking for NULL is good practice, however. ' statement, coz I think since it's possible to overload the new()/delete() operators it's not guaranteed that calling delete with a NULL pointer works.
0
 
LVL 2

Expert Comment

by:dhymes
ID: 2653120
Always check for NULL, it is a simple camparison of memory that equates to about 3 lines of assembler code and executes quickly. If you get into the habbit of not checking for NULL and build a LARGE application the you will need to get used to seeing the old "Memory Exception" error or "Segmentation Fault" depending on what OS you are coding on.
0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2653198
Zoppo .. if you write a bad new/delete operator that cannot handle a NULL pointer, then you're in trouble anyway.  And the best place to put the check is in the delete operator itself so that it appears in one place anyway.

dhymes .. delete NULL; is fine .. not OS dependant etc.. its part of the language spec.

You should always be checking for NULL before dereferencing a pointer eg.

if (p != NULL) {
  p->xxxx
}

but don't need to for

if (p != NULL) {
  delete p;
}

is a waste of two lines when

delete p;

is just as good.

I guess the only possible good thing about the NULL test is that it _may_ save time if you have a lot of null pointers you are trying to delete, as it avoids the function call (if any) to the delete operator code (maybe inline anyway) for a NULL pointer.

But I'd rather see shorter code with less unnecessary clutter.  And less lines mean less mistakes ... who knows one might be silly enough to accidentally type

if (p = NULL) {
  delete p;
}

or

if (p == NULL) {
  delete p;
}

etc.
0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2653223
Also, asa metter of god practice, don't write

if (p != NULL)
  delete p;

instead say

if (p != NULL) {
  delete p;
}

if you split an if (or for etc) over more than one line, then it is always good practice to use {}.  Otherwise it is sooo easy to make the mistake of later adding another line and indenting so it _looks_ correct, or using a macro that expands to more than one statement.  Such bugs are very easy to make and hard to find.

also you can shorten this to

if (p) {
  delete p;
}

the '!= NULL' is a waste of space, too

And another thing on style/practice .. when checking for NULL (or any other constant value), don't code

  if (n == 1) ...

instead get into the habit of

  if (1 == n) ...

so that if you accidentally use = instead of ==, the compiler will tell you that you made a mistake.

And still another, instead of

  if (p == NULL) ...

you can just say

  if (! p) ...

0
 
LVL 22

Expert Comment

by:nietod
ID: 2653258
I strongly agree with Ronslow, as does Bjarne Stroustrup and many members of the standards committe.  Not only is not necessary to check if a pointer is NULL before deleting, it is a waste of time and space

The delete operator will make the check, so there is no need to make the check twice.  If you make the check before calling the delete operator, you will save yourself the time of making a single function call, but that savings will occur very infrequently (and will not be significant even when it does occur because you will have to jump around the call instruction code, and on many CPUs is a conditional jump like that is almost as costly as an function call.)  But in most cases you won't save anything, you will use more CPU time, because after spending the time doing the test you will then make the call to the delete operator.  
0
Do You Know the 4 Main Threat Actor Types?

Do you know the main threat actor types? Most attackers fall into one of four categories, each with their own favored tactics, techniques, and procedures.

 
LVL 30

Expert Comment

by:Zoppo
ID: 2653270
>if you write a bad new/delete operator that cannot handle a NULL pointer...

It may happen that I have to use overloaded new/delete operators that I didn't implement myself
0
 
LVL 9

Expert Comment

by:Pacman
ID: 2653628
Do you guys verify each memory block allocated with new operator ?
Can't believe that.
My windows applications consist of hundreds to thousands of new's

I do only verify if the size is variable or very big.
0
 
LVL 2

Expert Comment

by:abesoft
ID: 2653646
Not meaning to nit, but RONSLOW made the statement:

>The argument that it was a hangover from c is not really valid, because free works fine with a NULL pointer as well.

This may be true in some implementations of the library, but I _know_ that it dumped core/faulted on others.  I believe that the standard claimed that the behaviour was undefined.
0
 
LVL 32

Expert Comment

by:jhance
ID: 2653740
It _IS_ good practice to check for calling delete on a NULL pointer.  Granted it's not going to cause a program crash.

The reason I mentioned that checking for NULL is that you need to know WHY you're calling delete with a NULL pointer.  It's almost always a programming error.  Forgetting to new the object, deleting it twice, failing to check for a failed new operation, etc.

Like I said, it's not necessary, but it's one of those things that you think will never happen, but always does when your customer runs the program.
0
 
LVL 22

Expert Comment

by:nietod
ID: 2653957
>> Do you guys verify each memory block
>> allocated with new operator ?
No point.  Unlike malloc() new throws an exception if it fails.

>> because free works fine with a NULL
>> pointer as well...
Yes, free() is not required to behave when passed a NULL pointer, and I've never seen a case where it did behave.  malloc() and free() handling of error conditions is beleived to be the single greatest sort of failures in C programs.  malloc() quitly returns errors (returns a NULL) which in the vast majority of cases neber gets tested for (though it shoudl be).  On the other  hand free() can't take a NULL.  Either one of these facts is an accident waiting to happen (not waiting long), take together they are even worse.  It is for this reason that the C++ standards committee debated long an hard on the propper behavior of new and delete, they wanted to avoid these sorts of errors.  the final decission was to have new throw an exception and to have delete work correctly when passed NULL.  In a properly designed program these two conditions work together very well.

> I mentioned that checking for NULL is that
>> you need to know WHY you're calling delete with
>> a NULL pointer
That is a different condition.  You are saying that if at some time a pointer should never be NULL, you should check that it is not NULL.  I agree with that.  Although I would say the check shoudl be an assert or similar debug-only check.  But that is a debugging check, you aren't going to ignore this "error condition", you are goign to report it.  It has nothing to do with checking before a delete.  There are many times when it is legitimate to have a pointer that might or might not be NULL and if it is not NULL you want to delete what it points to.  In this case the NULL is not in error.
0
 
LVL 10

Accepted Solution

by:
RONSLOW earned 10 total points
ID: 2656410
>>> Do you guys verify each memory block
>>> allocated with new operator ?
>No point.  Unlike malloc() new throws an exception if it fails.

I think the point being made here is that you need to have some faith in things working (until proven otherwise) .. there are things in the lanuage and your impplementation and library that you assume work as they should .. otherwise you'll end up writing pages andpages or error checking and validation code.  So, if the standard says that delete works with NULL, then it is reasonable and realistic to assume that it does.

>>> because free works fine with a NULL
>>> pointer as well...
>Yes, free() is not required to behave when passed a NULL pointer
I thought the C standard said free IS supposed to work correctly with a NULL (maybe that is only C++).  Guess we need someone with a C standard and check.

Of course, no compiler / implementation / library is perfect.

BUT .. If you _really_ want to protect against faulty compilers, implementations and libraries, then it is probably better and nicer to wrap all this up in your own function/macro and centralise the checking.  Putting code around every call to delete is wasteful.

eg. if your implementation of delete is faulty and cuases problems with NULL, then do this...

template <class T> inline void Delete(T* p) {
  if (p) delete p; // or whatever you need to do
}

and then use
  Delete (p);
instead of
  delete p;

That way you don't need to add the same checking code around every deallocation.

>>> I mentioned that checking for NULL is that
>>> you need to know WHY you're calling delete with
>>> a NULL pointer
>That is a different condition.  

Indeed .. error checking for pointers that _shouldn't_ be NULL should be just that ..

if (p == NULL) {
  .. throw and exception or whatever
}
delete p;

otherwise you're just ignoring the problem (and you'll never know that something went wrong elsewhere in your program to make your pointer NULL).

Again, I'd wrap this sort of checking in a function/macro.

0
 

Author Comment

by:perizi
ID: 2660588
Since delete operation accepts a NULL pointer, it is not necessary to check that pointer before calling delete.
Surprisingly, many programers prefer checking it.
I would say that almost all programmers do check for NULL before calling delete; That's why I wanted to know experts opinion.

After reading the comments, I've reinforced my opinion: it is a waste of space and time, although it gives a security feeling to the programmer.

JHANCE, Thanks for your answer and comments but I must reject your answer. I can't see that's a good practice. If I experience problems that make change my mind, I'll make all you know.

RONSLOW, I agree with you. Thanks for your extensive and clear explanations. Points are yours.
0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2662071
Thanks, and good luck .. hopefully all your NULL pointers will delete happily ever-after.

BUT (seriously) if they don't, try to wrap a workaround for the problem as tightly as possible in one place, rather than spread them throughout the code (eg. if delete doesn't work with NULL in your system, then wrapp the delete call in a marco/template and use that everywhere rather than the same if fixes around all your delete statments).

This if always good practice and is also recommended whenever you are forced to do something 'ugly' or machine specific.  Wrap it up in a function or a class so the ugliness is contained and the rest of your code remains a pristine work of art :-)

0

Featured Post

Better Security Awareness With Threat Intelligence

See how one of the leading financial services organizations uses Recorded Future as part of a holistic threat intelligence program to promote security awareness and proactively and efficiently identify threats.

Join & Write a Comment

  Included as part of the C++ Standard Template Library (STL) is a collection of generic containers. Each of these containers serves a different purpose and has different pros and cons. It is often difficult to decide which container to use and …
C++ Properties One feature missing from standard C++ that you will find in many other Object Oriented Programming languages is something called a Property (http://www.experts-exchange.com/Programming/Languages/CPP/A_3912-Object-Properties-in-C.ht…
The viewer will learn how to use the return statement in functions in C++. The video will also teach the user how to pass data to a function and have the function return data back for further processing.
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

706 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

17 Experts available now in Live!

Get 1:1 Help Now