Sign in to follow this  

Returning a de-referenced NULL

This topic is 4380 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Just found this little code snippet in the source code of irrlicht 0.1
//! returns the axis aligned bounding box of this node
const core::aabbox3d<f32>& CSceneManager::getBoundingBox() const
{
	#ifdef _DEBUG
	os::Warning::print("Bounding Box of Scene Manager wanted.");
	_asm int 3;
	#endif

	// should never be used.
	return *((core::aabbox3d<f32>*)0);
}


Now, wouldn't the return cause really bad things to happen since you are basically de-referencing a NULL pointer?

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
Dereferencing something involves actually using it for reading or writing to it. Here it's just a casted NULL pointer that is returned - but not used.

Share this post


Link to post
Share on other sites
As it's irrlicht 0.1 and there's a comment above the return statement 'should never be used' I'm guessing this method was just a place-holder until the author got round to actually writing the real version. As to why it returns a de-referenced null pointer it has to return a core::aabbox3d<f32>& and that's probably the simplest way to generate something to return. Also I think the actual return statement would execute without problems as a reference is basically a pointer in disguise. Problems would occur when you attempt to use the returned reference. Officially in the C++ standard the effect is almost certainly undefined though so pretty much anything could happen.

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
Dereferencing something involves actually using it for reading or writing to it. Here it's just a casted NULL pointer that is returned - but not used.


This code is an abomination. He's creating a null reference. People don't expect references to be invalid like that. Sure, returning it will work, but any attempt to use it will blow up. Have fun with that.

Quote:
and that's probably the simplest way to generate something to return.


If you look at the debugging string, it's trying to get the bounding box of an "abstract" object which doesn't have one. Why does that class possess such a member function? If it's to keep the class hierarchy simple, this is the right place to throw an exception. Deliberately tossing around null references is moronic. References aren't intended to be used as pointers.

Share this post


Link to post
Share on other sites
i think he simply has to return "something" to make compiler happy, and at this moment the class doesn't do anything other than throwing exception with "int 3".
But yes it's better to throw real exception.
Also i think it is bad idea to return reference to bounding box in first place.

Share this post


Link to post
Share on other sites
Of course, you do have to consider the fact that The Code Will Never Be Reached. If the function is entered AT ALL, it's an error - that's why theres a breakpoint in there. The only way to get to it is if the programmer enters the debugger.

Share this post


Link to post
Share on other sites
Quote:
Original post by Deyja
Of course, you do have to consider the fact that The Code Will Never Be Reached. If the function is entered AT ALL, it's an error - that's why theres a breakpoint in there. The only way to get to it is if the programmer enters the debugger.

nope. If _DEBUG is not defined, you get null reference.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
i think in a situation like this one you could also convince the compiler to shut up using __assure(false); .... evil code, i agree.

Share this post


Link to post
Share on other sites
This is definitely a place for an exception. If the compiler will complain about a missing return despite an unconditional 'throw' immediately preceeding it, I'd suggest returning a default-constructed object (or, if the default constructor doesn't do anything, a bounding box with the coordinates initialized to +/- infinity or NaN) for those truly odd "just in case" situations.

Share this post


Link to post
Share on other sites
Yes, that's how you would create a null reference. It most likely doesn't crash in that function because the dereference is cancelled out by the fact that you want to return a reference, which basically being a pointer, does the opposite of dereferencing (i.e. getting the address of it).

It's still an abomination though.

Share this post


Link to post
Share on other sites
Considering that this is still verion 0.1 and is probably still in production... looks like the author has gone to great lengths in avoiding people using this function!

Also, the int 3 is forcing people to use x86 machines for debugging! Whatever happened to ASSERT?

Everyone to his own...

Share this post


Link to post
Share on other sites
Quote:
Original post by Oluseyi
Quote:
Original post by Aryabhatta
Considering that this is still verion 0.1 and is probably still in production...

...one should stay away from versions 0.2 through 2.4?

Bad programmers write bad code. That's some bad code. There's bound to be more.


LOL :-)
I never said that the code was good (how in the world did you deduce that?). Anyway, if the author knows what he is doing and is probably going to change the code soon, you can say that the code is bad, but not that the author is a bad programmer.

Share this post


Link to post
Share on other sites
There's no reason to do something like that in the first place. If you're going to poison a function like that then why bother to even define it? Just leave out the definition and you can detect at link time that the function is unimplemented and not have to deal with the landmine at runtime.

Share this post


Link to post
Share on other sites
Quote:
Original post by SiCrane
There's no reason to do something like that in the first place. If you're going to poison a function like that then why bother to even define it? Just leave out the definition and you can detect at link time that the function is unimplemented and not have to deal with the landmine at runtime.


Unless we know the reason why the author put that in, we cannot claim that (s)he is a bad programmer. I don't think it is fair to the author to jump to such a conclusion based on one such snippet.

Share this post


Link to post
Share on other sites
Quote:
Original post by Aryabhatta
Quote:
Original post by SiCrane
There's no reason to do something like that in the first place. If you're going to poison a function like that then why bother to even define it? Just leave out the definition and you can detect at link time that the function is unimplemented and not have to deal with the landmine at runtime.


Unless we know the reason why the author put that in, we cannot claim that (s)he is a bad programmer.


Sure we can - or at least that they were. Stubbing out a function with undefined behavior like that is just silly.

Share this post


Link to post
Share on other sites
Heres my question:

The function still needs a return value to compile, even if you throw an exception, doesn't it. So the line of code with undefined behavior still needs to be there to make it compile, right? Or am I missing something...


Edit: Removed an assumption that was not true

Share this post


Link to post
Share on other sites
Quote:
Original post by SiCrane
Quote:
Original post by JBourrie
Ok, so we all agree that he should have thrown an exception...

No we don't.


I'm with SiCrane here. Leave out the function definition entirely. That way you get an error at link time, which is a shitload easier to fix than something randomly blowing up at runtime (exception or otherwise).

Share this post


Link to post
Share on other sites
Quote:
Original post by JBourrie
Heres my question:

The function still needs a return value to compile, even if you throw an exception, doesn't it.


Nope. VS raises an error if it can tell a funciton can reach the end without returning when it needs to - a warning if it might - and in the case where you're throwing instead - nada, because that's perfectly sane.

I'm all for linker errors, although sometimes stubs make sense. I don't consider that an either-or-only decision.

Share this post


Link to post
Share on other sites
"That way you get an error at link time."


Right, that's not my question. What if you don't want an error at link time, because you're not calling the function yet and you want the thing to actually compile? That's what it looks like this guy was doing, and so I'm not sure how you would otherwise handle this situation in certain circumstances (such as when building the object that you are returning a reference to is nontrivial).

@MaulingMonkey:

So in VS you can throw an exception instead of returning, and the compiler won't complain about a missing return statement? What about other compilers, though?

I'm just trying to figure out why people are being so hard on this obviously placeholder construct. If I were developing on GCC or something that didn't make the concessions to the language that VS does, is this really such a bad thing to do as a placeholder? Is there even an alternative?

Share this post


Link to post
Share on other sites
Quote:
Original post by JBourrie
Right, that's not my question. What if you don't want an error at link time, because you're not calling the function yet and you want the thing to actually compile?
It will compile fine if the function is never called. The linker will only look for the function's code if an actual call to it exists. Otherwise, the compiler is happy to have the function declaration as part of the class without an accompanying definition.

Share this post


Link to post
Share on other sites

This topic is 4380 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this