Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > if(!p) { assert(false); return NULL; } ... ?

Reply
Thread Tools

if(!p) { assert(false); return NULL; } ... ?

 
 
Martin B.
Guest
Posts: n/a
 
      09-27-2012
I'm curious what others think of this pattern:

+ + + +
TX* f(TY* p, ...) {
if (!p) {
assert(false);
return NULL;
}
...
}
+ + + +

I'm not sure whether I should accept it as defensive programming or
think it's just plain superfluous?!

So I have a function that is required to only accept a non-null pointer,
"documented" by the assert statement, only to have the function fail
"gracefully" should it be passed a NULL pointer. (Obviously, returning
NULL is *only* done in this case, otherwise a valid pointer will be
returned.)

cheers,
Martin


p.s.: While I'm at it, I'll give one of the reasons such things are
explained: Developer had a crash dump where the immediate crash reason
was dereferencing of `p` inside f when it was NULL. Since no one was
able to reproduce it in due time, they added the check "so it won't
crash anymore". Which, yes, it doesn't, but now we have this code and
even longer debugging sessions when the program doesn't do what it's
supposed to do (but doesn't crash).

--
I'm here to learn, you know.
Just waiting for someone,
to jump from the shadows,
telling me why I'm wrong.
 
Reply With Quote
 
 
 
 
Victor Bazarov
Guest
Posts: n/a
 
      09-27-2012
On 9/27/2012 1:59 PM, Martin B. wrote:
> I'm curious what others think of this pattern:
>
> + + + +
> TX* f(TY* p, ...) {
> if (!p) {
> assert(false);
> return NULL;
> }
> ...
> }
> + + + +
>
> I'm not sure whether I should accept it as defensive programming or
> think it's just plain superfluous?!


It's a hack. A proper way to handle the situation with non-acceptable
argument value would be to throw an exception, unless, of course, there
is a "nothrow" requirement from the client (caller)...

> So I have a function that is required to only accept a non-null pointer,
> "documented" by the assert statement, only to have the function fail
> "gracefully" should it be passed a NULL pointer. (Obviously, returning
> NULL is *only* done in this case, otherwise a valid pointer will be
> returned.)


Since we're not offered the specification, we can't judge how close this
code matches them, can we? Trying to divine the requirements from the
code hinges on the assumption that the code correctly implements them.
At the same time judging the code's correctness assumes that the
requirements (specification) is known independent from the code. So,
you got a chicken and an egg problem, AFAICT.

> cheers,
> Martin
>
>
> p.s.: While I'm at it, I'll give one of the reasons such things are
> explained: Developer had a crash dump where the immediate crash reason
> was dereferencing of `p` inside f when it was NULL. Since no one was
> able to reproduce it in due time, they added the check "so it won't
> crash anymore". Which, yes, it doesn't, but now we have this code and
> even longer debugging sessions when the program doesn't do what it's
> supposed to do (but doesn't crash).


That's why I called it a hack. Welcome to code maintenance hell!

V
--
I do not respond to top-posted replies, please don't ask
 
Reply With Quote
 
 
 
 
Tobias Müller
Guest
Posts: n/a
 
      09-28-2012
"Martin B." <(E-Mail Removed)> wrote:
> I'm curious what others think of this pattern:
>
> + + + +
> TX* f(TY* p, ...) {
> if (!p) {
> assert(false);
> return NULL;
> }
> ...
> }
> + + + +
>
> I'm not sure whether I should accept it as defensive programming or think
> it's just plain superfluous?!
>
> So I have a function that is required to only accept a non-null pointer,
> "documented" by the assert statement, only to have the function fail
> "gracefully" should it be passed a NULL pointer. (Obviously, returning
> NULL is *only* done in this case, otherwise a valid pointer will be returned.)
>
> cheers,
> Martin
>
>
> p.s.: While I'm at it, I'll give one of the reasons such things are
> explained: Developer had a crash dump where the immediate crash reason
> was dereferencing of `p` inside f when it was NULL. Since no one was able
> to reproduce it in due time, they added the check "so it won't crash
> anymore". Which, yes, it doesn't, but now we have this code and even
> longer debugging sessions when the program doesn't do what it's supposed
> to do (but doesn't crash).


If your function cannot handle NULL pointers, then it should probably take
a reference as parameter.
If you do this consequently, you only have to check for NULL where you know
that the pointer can really be NULL. And at that place it should be clear
how to handle it.

Tobi
 
Reply With Quote
 
goran.pusic@gmail.com
Guest
Posts: n/a
 
      09-28-2012
On Thursday, September 27, 2012 7:59:14 PM UTC+2, Martin T. wrote:
> I'm curious what others think of this pattern:
>
>
>
> + + + +
>
> TX* f(TY* p, ...) {
>
> if (!p) {
>
> assert(false);
>
> return NULL;
>
> }
>
> ...
>
> }
>
> + + + +
>
>
>
> I'm not sure whether I should accept it as defensive programming or
>
> think it's just plain superfluous?!
>
>
>
> So I have a function that is required to only accept a non-null pointer,
>
> "documented" by the assert statement, only to have the function fail
>
> "gracefully" should it be passed a NULL pointer. (Obviously, returning
>
> NULL is *only* done in this case, otherwise a valid pointer will be
>
> returned.)
>
>
>
> cheers,
>
> Martin
>
>
>
>
>
> p.s.: While I'm at it, I'll give one of the reasons such things are
>
> explained: Developer had a crash dump where the immediate crash reason
>
> was dereferencing of `p` inside f when it was NULL. Since no one was
>
> able to reproduce it in due time, they added the check "so it won't
>
> crash anymore". Which, yes, it doesn't, but now we have this code and
>
> even longer debugging sessions when the program doesn't do what it's
>
> supposed to do (but doesn't crash).


Congratulations, you, too, have been hit by Hoare's billion $ mistake! So that's 1000000001$ mistake now (and counting).

Unfortunately, such code is abound. I say it's a consequence of crap C programming. Well, poor design, really (it's unknown whether NULL is allowed ornot). It did emerge from the desire to protect from crashes, as post-scriptum says, but the hard cold truth is that someone had given up on finding abug. Worse yet, the code stays (forever, sadly ) because it's seen as expedient to sweep the bug under a rug.

In C, correct code for the above is:

TX* f(TY* p, ...) {
assert(p); // If this fires, we die horrible death

...
}

In C++, it's

retval f(TY& p)
{
....
}

What I do when confronted with such code, is to change parameter type into a reference and try to fix the mess. Unfortunately, it's prohibitive in large codebases.

Finally, I don't buy the defensive coding argument. To me, it's by far the best to crash right on the spot than to add piles of uncertainty like that.When you crash, you send a powerful signal that something is wrong, and are therefore obliged to fix that. When you "defend", you just make a bigger mess. It's much better to factor the crashes in early. That also forces youto think about protecting relevant code outputs on time, as well as havingmeans to debug crashes.

Goran.
 
Reply With Quote
 
Martin B.
Guest
Posts: n/a
 
      09-28-2012
On 28.09.2012 07:00, Gareth Owen wrote:
> "Martin B." <(E-Mail Removed)> writes:
>
>> I'm not sure whether I should accept it as defensive programming or
>> think it's just plain superfluous?!

>
> A constraint violation that fails gracefully in production builds
> (-DNDEBUG), and dies with a stack trace in debug builds?
>
> Given the other methods of obtaining a stack trace are a pain, and
> assuming the return value gets checked, I can't see anything to get
> worked up about.
>


Yes, but I wrote that the only way this function can return NULL is the
"impossible" case. Even if the return value is checked, all that *is*
done there is ignore the error further. Leading to a non-crashing, but
non-working application, without any info to the user.


--
Good C++ code is better than good C code, but
bad C++ can be much, much worse than bad C code.
 
Reply With Quote
 
Martin B.
Guest
Posts: n/a
 
      09-28-2012
On 28.09.2012 08:43, Tobias Müller wrote:
> "Martin B." <(E-Mail Removed)> wrote:
>> I'm curious what others think of this pattern:
>>
>> + + + +
>> TX* f(TY* p, ...) {
>> if (!p) {
>> assert(false);
>> return NULL;
>> }
>> ...
>> }
>> + + + +
>>
>> I'm not sure whether I should accept it as defensive programming or think
>> it's just plain superfluous?!
>> ...

>
> If your function cannot handle NULL pointers, then it should probably take
> a reference as parameter.
> If you do this consequently, you only have to check for NULL where you know
> that the pointer can really be NULL. And at that place it should be clear
> how to handle it.
>


I have to say the reference suggestion seem highly naive.

You can't rework a whole large code base, and changing this function
from a ptr to a ref would just shift the burden of checking for NULL to
the call site that now has to dereference a pointer to pass it to the
function.


--
Good C++ code is better than good C code, but
bad C++ can be much, much worse than bad C code.
 
Reply With Quote
 
Ike Naar
Guest
Posts: n/a
 
      09-29-2012
On 2012-09-28, Martin B. <(E-Mail Removed)> wrote:
> On 28.09.2012 08:43, Tobias M?ller wrote:
>> "Martin B." <(E-Mail Removed)> wrote:
>>> I'm curious what others think of this pattern:
>>>
>>> + + + +
>>> TX* f(TY* p, ...) {
>>> if (!p) {
>>> assert(false);
>>> return NULL;
>>> }
>>> ...
>>> }
>>> + + + +
>>>
>>> I'm not sure whether I should accept it as defensive programming or think
>>> it's just plain superfluous?!
>>> ...

>>
>> If your function cannot handle NULL pointers, then it should probably take
>> a reference as parameter.
>> If you do this consequently, you only have to check for NULL where you know
>> that the pointer can really be NULL. And at that place it should be clear
>> how to handle it.

>
> I have to say the reference suggestion seem highly naive.
>
> You can't rework a whole large code base, and changing this function
> from a ptr to a ref would just shift the burden of checking for NULL to
> the call site that now has to dereference a pointer to pass it to the
> function.


But now you assume that, when using call-by-reference,
the call site needs to dereference a pointer, which need not be the case.

With call-by-pointer, the situation might look like

void f(Type *arg) /* precondition: arg must not be NULL */
{
/* assure arg != NULL */
/* use *arg */
}

Type var = /* meaningful initialization */;
f(&var);

with call-by-reference this would look like

void f(Type &arg)
{
/* use arg */
}

Type var = /* meaningful initialization */;
f(var);

If this is the typical usage of f, using call-by-reference leads to
simpler code that does not need NULL checks, neither in f nor at
the call site.
 
Reply With Quote
 
Jorgen Grahn
Guest
Posts: n/a
 
      09-29-2012
On Sat, 2012-09-29, Ike Naar wrote:
> On 2012-09-28, Martin B. <(E-Mail Removed)> wrote:
>> On 28.09.2012 08:43, Tobias M?ller wrote:
>>> "Martin B." <(E-Mail Removed)> wrote:
>>>> I'm curious what others think of this pattern:
>>>>
>>>> + + + +
>>>> TX* f(TY* p, ...) {
>>>> if (!p) {
>>>> assert(false);
>>>> return NULL;
>>>> }
>>>> ...
>>>> }
>>>> + + + +
>>>>
>>>> I'm not sure whether I should accept it as defensive programming or think
>>>> it's just plain superfluous?!
>>>> ...
>>>
>>> If your function cannot handle NULL pointers, then it should probably take
>>> a reference as parameter.
>>> If you do this consequently, you only have to check for NULL where you know
>>> that the pointer can really be NULL. And at that place it should be clear
>>> how to handle it.

>>
>> I have to say the reference suggestion seem highly naive.
>>
>> You can't rework a whole large code base, and changing this function
>> from a ptr to a ref would just shift the burden of checking for NULL to
>> the call site that now has to dereference a pointer to pass it to the
>> function.

>
> But now you assume that, when using call-by-reference,
> the call site needs to dereference a pointer, which need not be the case.
>
> With call-by-pointer, the situation might look like
>
> void f(Type *arg) /* precondition: arg must not be NULL */
> {
> /* assure arg != NULL */
> /* use *arg */
> }
>
> Type var = /* meaningful initialization */;
> f(&var);
>
> with call-by-reference this would look like
>
> void f(Type &arg)
> {
> /* use arg */
> }
>
> Type var = /* meaningful initialization */;
> f(var);
>
> If this is the typical usage of f, using call-by-reference leads to
> simpler code that does not need NULL checks, neither in f nor at
> the call site.


Yeah. And what often happens when you do such transformations is, you
find that 95% of the call sites are non-problematic (f(&var)) and 5%
are the ones where you actually *do* have a potential NULL pointer.

If you can find a way to deal with that at the call site you've
improved the code a lot -- the unsafety is focused to fewer code paths,
and you're encouraging new code to be safe.

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .
 
Reply With Quote
 
Jorgen Grahn
Guest
Posts: n/a
 
      09-29-2012
On Thu, 2012-09-27, Martin B. wrote:
> I'm curious what others think of this pattern:
>
> + + + +
> TX* f(TY* p, ...) {
> if (!p) {
> assert(false);
> return NULL;
> }
> ...
> }
> + + + +


Seems like an odd way of saying:

TX* f(TY* p, ...)
{
assert(p);
if (!p) {
return NULL;
}
...
}

assert() works best if it asserts something meaningful.
"My bank account is not empty" instead of "if my bank account is
empty, the Moon is made of blue cheese".

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .
 
Reply With Quote
 
Martin B.
Guest
Posts: n/a
 
      09-30-2012
On 28.09.2012 22:42, Paavo Helde wrote:> "Martin B." <(E-Mail Removed)>
wrote in
> news:k44umq$edi$(E-Mail Removed):
>
>> On 28.09.2012 08:43, Tobias MCller wrote:

>
>>> If your function cannot handle NULL pointers, then it should probably
>>> take a reference as parameter.
>>> If you do this consequently, you only have to check for NULL where
>>> you know that the pointer can really be NULL. And at that place it
>>> should be clear how to handle it.
>>>

>>
>> I have to say the reference suggestion seem highly naive.
>>
>> You can't rework a whole large code base

>
> Why not? (...)


Someone has to pay for it.

> ... break the build; you just fix the call sites and hit the build button
> again; and again; and again. Yes, it may take long time for a really

large
> code base, but in the end everything is supposed to be cleaner. Been

there,

Yes, but your boss is going to ask you why he should pay fo it.

> done that, by introducing const-correctness to a large codebase where it
> was not taken into account seriously from the start.
>
> If you do not have a one-button build of the whole codebase, then

this is a
> real problem and must be dealt with first.
>


I have a one-button build. It just takes too long.

cheers,
Martin



 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
what value does lack of return or empty "return;" return Greenhorn C Programming 15 03-06-2005 08:19 PM
difference between return &*i and return i; Ganesh Gella C++ 4 11-12-2004 04:28 PM
getting return value from function without return statement. Seong-Kook Shin C Programming 1 06-18-2004 08:19 AM
How do I return a return-code from main? wl Java 2 03-05-2004 05:15 PM
Return a return value from Perl to Javascript PvdK Perl 0 07-24-2003 09:20 AM



Advertisments