tags:

views:

1111

answers:

5

Does it ever make sense to check if this is null?

Say I have a class with a method; inside that method, I check this == NULL, and if it is, return an error code.

If this is null, then that means the object is deleted. Is the method even able to return anything?

Update: I forgot to mention that the method can be called from multiple threads and it may cause the object to be deleted while another thread is inside the method.

+13  A: 

Does it ever make sense to check for this==null? I found this while doing a code review.

In standard C++, it does not, because any call on a null pointer is already undefined behavior, so any code relying on such checks is non-standard (there's no guarantee that the check will even be executed).

Note that this holds true for non-virtual functions as well.

Some implementations permit this==0, however, and consequently libraries written specifically for those implementations will sometimes use it as a hack. A good example of such a pair is VC++ and MFC - I don't recall the exact code, but I distinctly remember seeing if (this == NULL) checks in MFC source code somewhere.

It may also be there as a debugging aid, because at some point in the past this code was hit with this==0 because of a mistake in the caller, so a check was inserted to catch future instances of that. An assert would make more sense for such things, though.

If this == null then that means the object is deleted.

No, it doesn't mean that. It means that a method was called on a null pointer, or on a reference obtained from a null pointer (though obtaining such a reference is already U.B.). This has nothing to do with delete, and does not require any objects of this type to have ever existed.

Pavel Minaev
With `delete`, the opposite is usually true -- if you `delete` an object, it doesn't get set to NULL, and then you attempt to call a method on it, you'll find that `this != NULL`, but it will likely crash or behave oddly if its memory has already been reclaimed for use by some other object.
Adam Rosenfield
IIRC, the standard allows for `delete` to set the pointer to NULL, but does not require it, and in practice almost no implementations do so.
rmeador
`delete` might not always get an l-value though, consider `delete this + 0;`
GMan
Also, in C++ new is *supposed* to throw an exception on failure, not to return NULL (although many compilers including MSVC up to version 6 return NULL when new fails).
Adisak
Visual Studio 6 was released in June 1998. The C++ standard was published in September. OK, so Microsoft could have anticipated the standard, but in principle it is not surprising that many pre-standard compilers do not implement the standard ;-)
Steve Jessop
@Steve: The problem was you *couldn't* write standard-conforming C++ with msvc6 and that it was still used for so long after '98.
Roger Pate
+8  A: 

Your note about threads is worrisome. I'm pretty sure you have a race condition that can lead to a crash. If a thread deletes an object and zeros the pointer, another thread could make a call through that pointer between those two operations, leading to this being non-null and also not valid, resulting in a crash. Similarly, if a thread calls a method while another thread is in the middle of creating the object, you may also get a crash.

Short answer, you really need to use a mutex or something to synchonize access to this variable. You need to ensure that this is never null or you're going to have problems.

Tim Sylvester
"You need to ensure that this is never null" - I think a better approach is to ensure that the left operand of `operator->` is never null :) but aside from that, I wish I could +10 this.
Pavel Minaev
+1  A: 

FWIW, I have used debugging checks for (this != NULL) in assertions before which have helped catch defective code. Not that the code would have necessarily gotten too far with out a crash, but on small embedded systems that don't have memory protection, the assertions actually helped.

On systems with memory protection, the OS will generally hit an access violation if called with a NULL this pointer, so there's less value in asserting this != NULL. However, see Pavel's comment for why it's not necessarily worthless on even protected systems.

Michael Burr
There is still a case for asserts, AV or not. The problem is that an AV will usually not happen until a member function actually tries to access some member field. Quite often they just call something else (etc...), until eventually something crashes down the line. Alternatively, they can call a member of another class or a global function, and pass the (supposedly non-null) `this` as an argument.
Pavel Minaev
@Pavel: true enough - I softened my wording about the value of asserting `this` on systems with memory protection.
Michael Burr
actually, if assert is going to work, then definitely the main code also will work right?
Gokul
@Gokul: I'm not sure I fully understand your question, but even if the assert 'passes' you could get a bogus `this` pointer if your program is doing bad things with pointer arithmetic or maybe calling a member function of a class for an object composed inside embedded in another class through a NULL pointer for the 'outer' class. I hope that sentence made some sort of sense.
Michael Burr
A: 

I'd also add that it's usually better to avoid null or NULL. I think the standard is changing yet again here but for now 0 is really what you want to check for to be absolutely sure you're getting what you want.

Charles Eli Cheese
A: 

Your method will most likely (may vary between compilers) be able to run and also be able to return a value. As long as it does not access any instance variables. If it tries this it will crash.

As others pointed out you can not use this test to see if an object has been deleted. Even if you could, it would not work, because the object may be deleted by another thread just after the test but before you execute the next line after the test. Use Thread synchronization instead.

If this is null there is a bug in your program, most likely in the design of your program.

Carsten