use assert and defensive together

[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,

[s0suk3@gmail.com]

Re: use assert and defensive together

On Sep 22, 9:25 pm, bingfeng <bfzhao@gmail.c omwrote:

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

[Barry Schwarz]

Re: use assert and defensive together

On Mon, 22 Sep 2008 19:25:31 -0700 (PDT), bingfeng <bfzhao@gmail.c om>
wrote:

>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?

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

--
Remove del for email

[Peter Nilsson]

Re: use assert and defensive together

bingfeng <bfz...@gmail.c omwrote:

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.

--
Peter

[Peter Nilsson]

Re: use assert and defensive together

Barry Schwarz <schwa...@dqel. comwrote:

bingfeng <bfz...@gmail.c omwrote:

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

>
Can this expression ever evaluate to true?

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

   {
       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?

Ditto.

   }
   ...
}

--
Peter

[bingfeng]

Re: use assert and defensive together

On Sep 23, 10:52 am, s0s...@gmail.co m wrote:

On Sep 22, 9:25 pm, bingfeng <bfz...@gmail.c omwrote:
>
>
>

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!).
>

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

[William Pursell]

Re: use assert and defensive together

On 23 Sep, 03:25, bingfeng <bfz...@gmail.c omwrote:

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.

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.

[Richard]

Re: use assert and defensive together

William Pursell <bill.pursell@g mail.comwrites:

On 23 Sep, 03:25, bingfeng <bfz...@gmail.c omwrote:

>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.

>
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".

[bingfeng]

Re: use assert and defensive together

On Sep 23, 11:50 am, William Pursell <bill.purs...@g mail.comwrote:

On 23 Sep, 03:25, bingfeng <bfz...@gmail.c omwrote:
>
>
>

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.

>
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.

[Peter Nilsson]

Re: use assert and defensive together

William Pursell <bill.purs...@g mail.comwrote:

bingfeng <bfz...@gmail.c omwrote:

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.

>
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.

--
Peter

[Barry Schwarz]

Re: use assert and defensive together

On Mon, 22 Sep 2008 20:15:47 -0700 (PDT), Peter Nilsson
<airia@acay.com .auwrote:

>Barry Schwarz <schwa...@dqel. comwrote:

>bingfeng <bfz...@gmail.c omwrote:

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

>>
>Can this expression ever evaluate to true?

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

   {
       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?

>
>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.

>

   }
   ...
>}

--
Remove del for email

[bingfeng]

Re: use assert and defensive together

On Sep 23, 12:27 pm, bingfeng <bfz...@gmail.c omwrote:

On Sep 23, 11:50 am, William Pursell <bill.purs...@g mail.comwrote:
>

On 23 Sep, 03:25, bingfeng <bfz...@gmail.c omwrote:

>

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.

>

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.

I'm confused by myself, they are not typos. some should not be NULL.
sorry for that

>

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.

[Richard Heathfield]

Re: use assert and defensive together

bingfeng said:

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

ITYM assert(some != NULL);

if (!some)
{
return -1; //-1 indicates error
}

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; }

...
}
>
2.
int foo (const char* some)
{
if (!some)
{
assert(some);

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.

--
Richard Heathfield <http://www.cpax.org.uk >
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999

[Nick Keighley]

Re: use assert and defensive together

On 23 Sep, 05:05, Richard<rgr...@ gmail.comwrote:

William Pursell <bill.purs...@g mail.comwrites:

On 23 Sep, 03:25, bingfeng <bfz...@gmail.c omwrote:

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.

>

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.

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

[James Kuyper]

Re: use assert and defensive together

Peter Nilsson wrote:

William Pursell <bill.purs...@g mail.comwrote:

>bingfeng <bfz...@gmail.c omwrote:

....

>>int foo (const char* some)
>>{
>> assert(some);

....

> In particular, "assert( some )", which should be re-
>written as "assert( some != NULL )",

>
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.