use assert and defensive together

B

bingfeng

hello,
please see following two code snatches:
1.
int foo (const char* some)
{
assert(some);
if (!some)
{
return -1; //-1 indicates error
}
...
}

2.
int foo (const char* some)
{
if (!some)
{
assert(some);
return -1; // -1 indicates error
}
...
}

what’s the preference of you or none of them? why? I prefer the second
but I cannot persuade my friends.

regards,
 
S

s0suk3

hello,
please see following two code snatches:
1.
int foo (const char* some)
{
    assert(some);
    if (!some)
    {
        return -1; //-1 indicates error
    }
    ...

}

2.
int foo (const char* some)
{
    if (!some)
    {
        assert(some);
        return -1; // -1 indicates error
    }
    ...

}

what’s the preference of you or none of them? why? I prefer the second
but I cannot persuade my friends.

They both will do the same thing regardless of whether 'some'
evaluates to true or false, so it's only a matter of style. I'd say I
prefer the first; the second seems like an abuse of the assert()
macro, since it will always terminate the program (you already know it
will!).

Sebastian
 
B

Barry Schwarz

hello,
please see following two code snatches:
1.
int foo (const char* some)
{
assert(some);
if (!some)

Can this expression ever evaluate to true?
{
return -1; //-1 indicates error
}
...
}

2.
int foo (const char* some)
{
if (!some)
{
assert(some);
return -1; // -1 indicates error

Can this statement ever be reached?
 
P

Peter Nilsson

bingfeng said:
hello,
please see following two code snatches:
1.
int foo (const char* some)
{
    assert(some);
    if (!some)
    {
        return -1; //-1 indicates error
    }
    ...
}

2.
int foo (const char* some)
{
    if (!some)
    {
        assert(some);
        return -1; // -1 indicates error
    }
    ...
}

what’s the preference of you or none of them? why?

I generally prefer to test function preconditions
immediately on entry.

I also believe that asserts should be left in production
code. Otherwise your development environment is not
matching your production environment and you introduce
an element of risk in testing.

If this is 'critical' code that cannot aford to abort,
then there's even more reason not to introduce
differences between production and development
environments. [In other words, dump the asserts altogether.]
I prefer the second but I cannot persuade my friends.

You haven't said why you prefer it?

I don't think I would like having to sift through the
whole body of a function to make sure I've found all
the precondition asserts (should they change.) That
said, asserts are powerful things. If you change one,
you should review them all.
 
B

bingfeng

They both will do the same thing regardless of whether 'some'
evaluates to true or false, so it's only a matter of style. I'd say I
prefer the first; the second seems like an abuse of the assert()
macro, since it will always terminate the program (you already know it
will!).

Indeed, I think assert and defensive style is conflict here. General,
by using assert, you tell reader that the condition will not occur in
any case, so if sentence is not necessary. Obey the contract between
this function and client codes is the responsibility of
client. This requires you can control the client codes or the owner of
client codes has a strict contract with you. Of course, if the input
is from untrusted source, you'll definitely check every arguments you
get and make sure it's correct. assert is not for this scenario.

So I never use assert and defensive checking together. I prefer second
since I was forced select one from them. Apart from documenting codes
and asserting contact, assert can raise explicit break under debug
mode if possible, which give a very good change to check at once. This
maybe acceptable under some conditions of early stage of development.
Sebastian

bingfeng
 
R

Richard

William Pursell said:
Both of those code snippets are atrocities. In the first, the
assertion is a statement of expectation on the part of
the function developer. In particular, "assert( some )",
which should be re-written as "assert( some != NULL )",
is a proclamation that the function expects the caller
not to pass in NULL, and if the caller does then anything
may happen. In particular, the function will not return
a useful error condition and may invoke undefined
behavior, etc. By then checking for NULL, the developer
is essentially wasting runtime doing things that (s)he
promised would not be done. Either check for a condition,
or make the caller do it by making an assertion. Do not
do both.

The 2nd case is much worse, though. It is the developer
saying, "At this point in the code, I believe it is
logically impossible that the variable some has
a NULL value." Since it is inside a condition which
can only be reached when some is NULL, the developer
is clearly using a very powerful hallucinogenic.

Interesting as I had only quickly glanced at this before.

I have found in the past that sprinkling "asserts" around code tends to
promote an ill founded sense of "good practice" and smug satisfaction in
"safe code".

Never having been a fan of asserts I find this thread a good example of
this. The assert usage was not questioned and is only assumed "to be
good".
 
B

bingfeng

Both of those code snippets are atrocities. In the first, the
assertion is a statement of expectation on the part of
the function developer.  In particular, "assert( some )",
which should be re-written as "assert( some != NULL )",
is a proclamation that the function expects the caller
not to pass in NULL, and if the caller does then anything
may happen.  In particular, the function will not return
a useful error condition and may invoke undefined
behavior, etc.  By then checking for NULL, the developer
is essentially wasting runtime doing things that (s)he
promised would not be done.  Either check for a condition,
or make the caller do it by making an assertion.  Do not
do both.
yes, agree. should be assert(!some) both, typo.
The 2nd case is much worse, though.  It is the developer
saying, "At this point in the code, I believe it is
logically impossible that the variable some has
a NULL value."  Since it is inside a condition which
can only be reached when some is NULL, the developer
is clearly using a very powerful hallucinogenic.

Not only do we use assert to document impossibility and contract,
assert has another practical function to catch unexpected parameter
immediately. Say two groups of a team develop different components of
a big application and the both two components have interfaces to
other. They must do defensive check before use any value another
module pass in since they don't want to crash. However, an invalid
value means other module maybe in a incorrect condition, if they can
catch such potential bug during debug process, it's very helpful.
assert will give an explicit break and it's better than log and check
the message later. Maybe rewrite the second assert(some) as assert(!
some && "Invalid parameter from caller X") is more explicit. It's
*acceptable* in practice, IMO.
 
P

Peter Nilsson

William Pursell said:
Both of those code snippets are atrocities.  In the
first, the assertion is a statement of expectation on
the part of the function developer.

Any assert in a function body is such a statement.
 In particular, "assert( some )", which should be re-
written as "assert( some != NULL )",

For C90 conformance certainly.
is a proclamation that the function expects the caller
not to pass in NULL,

No, it's merely a proclamation that if NDEBUG is not
defined, the program will abort if given a null pointer.
Assert is a debugging tool. It's the requirements
specification that contains the proclamations.
and if the caller does then anything may happen.

Developers often write additional development code to
facilitate finding bugs. [E.g. checking linked lists
for loops, etc...] Such code needn't always see
the light of production.
 In particular, the function will not return
a useful error condition and may invoke undefined
behavior, etc.

It seems clear the two samples wish to avoid UB
whether the assert passes or fails.
 By then checking for NULL, the developer
is essentially wasting runtime doing things that (s)he
promised would not be done.

Developers do waste a lot of runtime. ;)
 Either check for a condition,
or make the caller do it by making an assertion.

Assertions do not force the caller to check the condition.
 Do not do both.

I agree, but not for the reasons you state.
The 2nd case is much worse, though. It is the developer
saying, "At this point in the code, I believe it is
logically impossible that the variable some has
a NULL value."

Again, it's merely a statement that if NDEBUG is not
defined, the program will abort if given a null pointer.
This may be deemed acceptable in a development environment,
but not in production.
 Since it is inside a condition which
can only be reached when some is NULL, the developer
is clearly using a very powerful hallucinogenic.

No, IMO they've simply made a poor design decision that
introduces unnecessary risks.
 
B

Barry Schwarz

Yes, when NDEBUG is defined and a null pointer is passed to foo.


Ditto.

In which case the two blocks of code are identical so there is no
basis for preferring one over the other which was the question in the
original post.
 
N

Nick Keighley

Interesting as I had only quickly glanced at this before.

I have found in the past that sprinkling "asserts" around code tends to
promote an ill founded sense of "good practice" and smug satisfaction in
"safe code".

Never having been a fan of asserts I find this thread a good example of
this. The assert usage was not questioned and is only assumed "to be
good.

I consider assert()s to be another useful tool in the programmer's
armoury. To an extent it makes the code "self-checking". I was
initially skeptical (partly because it looked "too simple")
but I gave it a try and became convinced of its utility.

But yes, sprinkling the magic assert dust over crappy code
doesn't make "good code".


--
Nick Keighley

... it is absurd to make elaborate security checks on
debugging runs, when no trust is put in the results, and
then remove them in production runs, when an erroneous
result could be expensive or disastrous.
C. A. R. Hoare
 
J

James Kuyper

Peter said:
For C90 conformance certainly.

Could you explain that? I don't believe that this is a conformance
issue, nor am I aware of any relevant changes between C90 and C99. As
far as I know, the only difference is in the clarity of the diagnostic
message produced if the assertion fails.
 
R

Richard Tobin

For C90 conformance certainly.
[/QUOTE]
Could you explain that? I don't believe that this is a conformance
issue, nor am I aware of any relevant changes between C90 and C99.

In C90, the synopsis for assert() is

void assert(int expression);

In C99, it's

void assert(scalar expression);

-- Richard
 
F

Flash Gordon

Richard Heathfield wrote, On 23/09/08 08:06:
bingfeng said:


ITYM assert(some != NULL);


If the function could conceivably receive a null pointer without this
meaning that the program has a bug somewhere, then the assertion should
not be there. If it could not, the conditional should not be there, for
the same reason that we don't write: x = 6; if(x < 0) { return -1; }

Depends on the complexity of what has gone before and.or where the
function is. It could be a function in the library which is documented
as requiring a non-null pointer, and the debug version of the library
causes an abort on failure but they have chosen for safety to also trap
it (but not as hard) in the release version.
Again, ITYM assert(some != NULL);

Here, it's simply pointless to put the assertion in place, since it is
inevitably going to fire. If you know the code here is wrong, why bother
to assert it? Just fix it.

I disagree that it is pointless to have both the assert and the if. The
assert is to immediately flag the problem during development if an
incorrect value is passed. The "if" is so that if the return value of
the function is handled correctly the program can recover (or maybe
abort tidily) if there is a coding error that causes it to trigger.
Sometimes it is a good thing to have both belt and braces, since when
the belt doesn't keep your trousers up the braces do!
 
K

Keith Thompson

If I were going to compare any versions, for #2 I'd have done just
assert(0), so that I wasn't duplicating the condition (some != NULL).

assert() write the text of its argument in the error message, so
assert(0) isn't going to be very informative (though it will also
print the file name and line number).
 
F

Flash Gordon

Malcolm McLean wrote, On 23/09/08 22:06:
The rationale for assert is completely opposite to that of defensive
programming.

I disagree.
assert terminates the program if an error occurs. defensive programming
tries to recover from the error.
Yes.

Which is better depends on the application.

If you want to abort on certain classes of error in the production
version then assert is the *wrong* thing to use. Under some conditions
aborting *is* a defensive action. For example with the SW I work on
regularly these days the software aborting is a minor annoyance, but
having the SW trying to continue when a problem is detected and writing
incorrect data to the database could lead to *major* costs for either
our customers or for us.
If you are writing softaware
for a medical life support machine you probably want to crash out on
error

Wrong. You want to try the two alternative methods and use the correct
result.
- this is unlikely to be any more disruptive than the machine
blowing a fuse, and the hospital will have procedures to deal with
failure.

There is a lot of work done to ensure that *won't* happen.
If you are writing software for a space suit life support
system, you want to hide the error and continue.

Wrong. You want to know there is an error and use the result from an
alternate implementation instead.
In orbit, there's no
way the astronaut can change suits, so its a choice between certain
death and possible death, and you choose possible death.

Actually, you provide alternate systems to keep the astronaut alive.

The safety critical system I worked on was not for astronauts, nor were
any of the other software that we carefully avoided being safety
critical by ensuring there were other systems that would prevent danger.
Generally defensive programming is poor practise.
Wrong.

It makes bugs
difficult to find.

Wrong. If you are debugging you can easily trap on the defensive action
being taken. This can be a breakpoint in a debugger if that is your
preferred method, or writing to a log file or anything else.
There is an argument for using both, and defining assert() to do nothing
in the production environment. So the natural way to hadle this is to
place the asserts at the top of the function, and the defensive control
logic below it.

At last something I can agree with.
 
K

Keith Thompson

The difference between

assertion failed, file "foo.c", line 123, "some != NULL"

and

assertion failed, file "foo.c", line 123, "0"

seems pretty slight to me, as far as making sense of it without looking at
the indicated file and line.

Sure, if you name your variable "some". If you actually give it a
*meaningful* name, the difference could be significant, at least in
terms of how long it takes you to figure out what the real problem is.
 
P

Peter Nilsson

Barry Schwarz said:
In which case the two blocks of code are identical

Semantically, yes. Stylistically, no.
so there is no basis for preferring one over the
other

There's no basis for preference on style?!
which was the question in the original post.

And I gave my preference: the code is clearer and
maintenance is simpler if function preconditions
appear at the start of the function body.
 
F

Flash Gordon

Malcolm McLean wrote, On 24/09/08 10:42:
Depends what you mean by "defensive programming". Generally aborts,
whether caused by segfaults or assert fails or exits, are not considered
to be defensive. You could argue that exit should be treated differently
because a correct program may call exit(EXIT_FAILURE).

I stated above why in this case it *is* defensive. It defends against
the database being corrupted.
No, as soon as the program is incorrect you've lost control.

Not always.
You want to
tell a human that the program can no longer be guaranteed to produce
correct results. Usually the simplest way to do this is to terminate.

Without additional systems aborting the program does *not* notify a
human. The human is likely at best sat at a desk several yards away not
watching the kit.
The exception is when no back-up can be available, in which case
defensive programming is appropriate - suppress the error, and hope.

Trigger the alarm. Or with the night vision system for the plane have
some night vision goggles in the cockpit and a radar altitude warning
system.
In fact life support machines are alarmed.

Yes, and that alarm plus a human within hearing range is one of the
backup systems! One that can always outvote the main system!
It's difficult to provide a
guaranteed continuous power supply, for instance. Hardware components
fail, people pull out plugs.

I'm fully aware of that. This is why not all systems are software
systems. Some are procedures humans are required to follow, some are
humans auditing that humans are following the procedures. Some are
hardware (for instance the monitoring of the heart rate not being simply
the same software controlling the breathing).
That was tried - triplicate control systems with a majority of two vote
on error. You still need to suppress errors in the individual threads,
or soon you've got a duplicate system and no possibility of a majority
vote. It didn't work well, largely because the software tended to fail
under the same circumstances.

The alternate system in this case can be a separate oxygen tank with a
mechanical control that has enough oxygen to keep the astronaut alive
while being rescued. This is what scuba divers do, they have a small
emergency tank which is independent from the main system.
If you are sitting at the debugger testing the defensively-written
function, yes.

You don't need to be. Notice the references to log files above.
Consider this

void defensivewriteimage(void *rgb, int width, int height)
{
if(width <= 0 || height <= 0)
return;
writeimage(rgb, width, height);
}

Great. test the code and it works as specified. Then two years later
someone calls it with a negative width, and no image is saved. The
program continues, and the user wonders why no image appears on his
disk.

That is not hard to debug. Stick a breakpoint on the return and you
catch it immediately. Or, as I suggested above, put in logging and look
at the logs. Or if it is an interactive program add in the code to
report the problem to the user.
Sometimes that's what you want. However it its a program for
internal use, the bug will come out if you replace the defensive if()
with an assert().

It does not come out on the production version if you do that. On the
production version you overwrite your original version of the image file
with some garbage and have your customer even more annoyed than if it
had simply failed to save the changed image.

In any case, it is easy to debug with one breakpoint or the addition of
logging or other reporting on the defensive action.
With the defensive programming, it's a long bughunt.

No, it isn't. I've been there with more complex scenarios than that.
Before I added the defensive code it was hard, I added the defensive
code and it managed to work well enough for the user whilst *also*
giving the developer (not always me) the necessary information to know
*exactly* what to look for in the code.
As for writing to a log file, that's fine if you have only one program
with an error-logging system set up.

It's fine with lots of them as well.
But it renders our function
inherently unreusable.

Ah, so the contracts my company has for lots of money are to provide
software built on inherently unreusable libraries. Unreusable libraries
that are used in different software written and maintained by other
companies.
Again, sometime that doesn't matter, but often
you need modular components.

It is *easy* to provide logging mechanisms that don't break modularity.

t_logfunc libwhatever_registerlog(t_logfunc logfunc);

Only one of many ways.
Generally defensive programming is a bad idea.

Wrong. It aids in debugging (the defensive code provides good hooks if
done properly) and
You want to terminate on
bugs.

Not always.

Another possible safety related (or even critical) system. The night
vision system for an aircraft. There is a bug in the image processing
software. Do you want the imaging to stop and not be able to see where
you are going or do you want a slightly corrupted image?
Even a segfault is often preferable to wrong results.

Sometimes it is, sometimes it isn't. If aborting the program on a
particular type of detected problem is the best thing to do then
aborting it *with* an appropriate message is the action you take.

Oh, and for a daemon I have worked on one of the defensive actions is
tracking a segfault and trying to send an email if one occurs to allow
the administrator to deal with it before the users know there is a problem.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top