You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stuart Celarier <SC...@corillian.com> on 2006/02/14 04:30:30 UTC

RE: assert() vs. return(error)

Julian wrote:
>Let's try to keep this discussion about the use of assertions separate
from >the thread about one particular instance.  To that end, I've
changed the >Subject line.

Brilliant, thanks.
 
>The debate on the relative merits and appropriate usage of assert vs.
error
>return is tricky.

All of the literature on assert (per se) seems to be mostly irrelevant
because the Subversion code uses assert in a non-standard way [1].

The rest of the C world uses the assert macro to compile assertions in
to the debug version, and to remove assertions in the release version.
Subversion always compiles with assertions in the code, including the
release version.

For Subversion, assert means "conditional abort."

I haven't seen another software project change the meaning of assert in
this way, so I don't think much of the existing writing on assert
applies to Subversion.

Best regards,
Stuart

Stuart Celarier | Corillian Corporation


[1] Thread starts at http://svn.haxx.se/dev/archive-2004-11/0421.shtml



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: assert() vs. return(error)

Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> Christian Stork wrote:
>> On Thu, Feb 16, 2006 at 03:17:02AM +0100, Branko Čibej wrote:
>>> Furthermore it's quite likely that a bug reported against the release
>>> version can't be reproduced in the debug version.
>>
>> How can this be if assertions don't have side-effects?  Are you talking
>> about timing issues?  If so, tough luck.  The bug needs to be debugged
>> without assert()s enabled.
> 
> Even if an assert has no side effects, disabling it could expose a 
> different bug, by allowing code to execute that would never have been 
> reached while the assert is active.
> 
> This is one of the QA nightmare scenarios:
> 
>    QA: Hey, we found a bug in the release bits. Here's the repro.
>    Dev: Sorry, can't reproduce your error, I hit an assert as expected.

No.  Dev: "Thanks for the recipe.  When I do that, instead of the bad behaviour 
that you observed I get an assertion failure which immediately confirms that 
there is a bug and makes it relatively easy to track down."

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: assert() vs. return(error)

Posted by Arlie Davis <ad...@stonestreetone.com>.
> This is one of the QA nightmare scenarios:
> 
>    QA: Hey, we found a bug in the release bits. Here's the repro.
>    Dev: Sorry, can't reproduce your error, I hit an assert as expected.
>    QA: So test the release version!
>    Dev: Uhuh. We don't do that.

The solution to this is simple.  Assertion failures are product failures.
If an assertion failure occurs, the offending code is broken, and must be
fixed.  The assertion must be analyzed, and if it is found to be bogus (the
condition can legitimately be false), then the assertion must be removed, or
it must take into account the legitimate false condition.

If the assertion is found to be valid, and the assertion fails, then the
developer *must* fix the code.  If a developer on my team said to me "I hit
an assert as expected", then I would fire (or re-educate) that developer.

I am of the mindset that assertions should be used to catch serious, fatal
flaws in flow control, data structure integrity, etc.  A failed assertion
means the product has gone completely off the rails.  It is not a short-cut
for a breakpoint.

-- arlie



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Branko Čibej <br...@xbc.nu>.
Christian Stork wrote:
> On Thu, Feb 16, 2006 at 03:17:02AM +0100, Branko Čibej wrote:
>   
> The reason why QA is done with disabled assert()s is that assert()s
> could be very inefficient and performance is certainly part of the Q
> that is being A'ed.  Another reason why assert()s shouldn't be in the
> final product is that the assert()s proper could be incorrect (this
> happened much more often to me than I would have expected) and you don't
> want your final product crash because of a false assert.
>   
Um. What you call a "require()" could also be incorrect. I fail to see 
the difference between assert() and require() here.

That's the main reason to test stuff -- because bits could be incorrect. :)

>> Furthermore it's quite likely that a bug reported against the release
>> version can't be reproduced in the debug version.
>>     
>
> How can this be if assertions don't have side-effects?  Are you talking
> about timing issues?  If so, tough luck.  The bug needs to be debugged
> without assert()s enabled.
>   
Even if an assert has no side effects, disabling it could expose a 
different bug, by allowing code to execute that would never have been 
reached while the assert is active.

This is one of the QA nightmare scenarios:

    QA: Hey, we found a bug in the release bits. Here's the repro.
    Dev: Sorry, can't reproduce your error, I hit an assert as expected.
    QA: So test the release version!
    Dev: Uhuh. We don't do that.

>> This debug- vs. release-mode dichotomy comes from the commercial world 
>> and its paranoia about reverse engineering. Here, we *like* reverse 
>> engineering.
>>     
>
> I have no idea where this comes from.  Can you give some more details?
> I don't know anything about real-world reverse engineering but I'm not
> sure that "effectless" assertion code might not even confuse the reverse
> engineer.
>   
Another common difference between debug and release versions is that the 
latter doesn't contain debug information.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Christian Stork <cs...@ics.uci.edu>.
On Thu, Feb 16, 2006 at 03:17:02AM +0100, Branko Čibej wrote:
> Christian Stork wrote:
> >Assertions are defined by their use.  Normally, that means that they act
> >as run-time aborts in the debugging version when the asserted condition
> >isn't met.  The shipped version of the code is compiled without
> >assertions.

> >Implications of this definition:

> >- assertions act as checked documentation
> >- assertions should not have side-effects
> >- assertions don't need to be very efficient

> >It seems that subversion and others would like to have something like
> >checked documentation combined with a conditional abort and use assert()
> >for that.  Maybe a different name like require() would be a better
> >choice, since it prevents confusion with the intended usage of C's
> >assert().

> >The folklore argument in favor of assertions over documentation is, IIRC:
> >"Assertions catch the error as early as possible."  I might add
> >"...while debugging."

> I really can't agree about the "while debugging" bit. IMNSHO 
> "development" and "release" versions should behave identically, 
> otherwise you have to do all your QA twice. 

Note that in my mail I neither argued for or against assertions in the
above sense.  I just said that assertions using C's assert() macro are
meant to "catch the error as early as possible ...while debugging."  And
I justify the "...while debugging" bit by looking at their intended
usage.  You seem to argue that one should not use assert() but only
require(), which is a different subject.

Now, on this different subject I disagree with you. :-)  QA should be
done on the code without assert()s (but with require()s)!  Of course, 
developers run the test suite with assert()s enabled, too.

The reason why QA is done with disabled assert()s is that assert()s
could be very inefficient and performance is certainly part of the Q
that is being A'ed.  Another reason why assert()s shouldn't be in the
final product is that the assert()s proper could be incorrect (this
happened much more often to me than I would have expected) and you don't
want your final product crash because of a false assert.

> Furthermore it's quite likely that a bug reported against the release
> version can't be reproduced in the debug version.

How can this be if assertions don't have side-effects?  Are you talking
about timing issues?  If so, tough luck.  The bug needs to be debugged
without assert()s enabled.

> This debug- vs. release-mode dichotomy comes from the commercial world 
> and its paranoia about reverse engineering. Here, we *like* reverse 
> engineering.

I have no idea where this comes from.  Can you give some more details?
I don't know anything about real-world reverse engineering but I'm not
sure that "effectless" assertion code might not even confuse the reverse
engineer.

-- 
Chris Stork   <>  Support eff.org!  <>   http://www.ics.uci.edu/~cstork/
OpenPGP fingerprint:  B08B 602C C806 C492 D069  021E 41F3 8C8D 50F9 CA2F

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Branko Čibej <br...@xbc.nu>.
Christian Stork wrote:
> Assertions are defined by their use.  Normally, that means that they act
> as run-time aborts in the debugging version when the asserted condition
> isn't met.  The shipped version of the code is compiled without
> assertions.
>
> Implications of this definition:
>
> - assertions act as checked documentation
> - assertions should not have side-effects
> - assertions don't need to be very efficient
>
> It seems that subversion and others would like to have something like
> checked documentation combined with a conditional abort and use assert()
> for that.  Maybe a different name like require() would be a better
> choice, since it prevents confusion with the intended usage of C's
> assert().
>
> The folklore argument in favor of assertions over documentation is, IIRC:
> "Assertions catch the error as early as possible."  I might add
> "...while debugging."
>   
I really can't agree about the "while debugging" bit. IMNSHO 
"development" and "release" versions should behave identically, 
otherwise you have to do all your QA twice. Furthermore it's quite 
likely that a bug reported against the release version can't be 
reproduced in the debug version.

This debug- vs. release-mode dichotomy comes from the commercial world 
and its paranoia about reverse engineering. Here, we *like* reverse 
engineering.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Christian Stork <cs...@ics.uci.edu>.
Assertions are defined by their use.  Normally, that means that they act
as run-time aborts in the debugging version when the asserted condition
isn't met.  The shipped version of the code is compiled without
assertions.

Implications of this definition:

- assertions act as checked documentation
- assertions should not have side-effects
- assertions don't need to be very efficient

It seems that subversion and others would like to have something like
checked documentation combined with a conditional abort and use assert()
for that.  Maybe a different name like require() would be a better
choice, since it prevents confusion with the intended usage of C's
assert().

The folklore argument in favor of assertions over documentation is, IIRC:
"Assertions catch the error as early as possible."  I might add
"...while debugging."

The problem is that assertions are often misused as conditional aborts.
But that's a straw man if a project with reasonable discipline decides
to use them properly (or not at all) and Subversion seems to have a lot
of discipline. ;-)

(Note that the fact that assertions should not have side-effects is only
checked if the program behaves the same with assertions enabled and
disabled.  If the program is never run without assertions side-effects
can creep into the asserted conditions without ever being noticed.  This
then is more confusing to the reader of the code.)

On Tue, Feb 14, 2006 at 07:57:07PM +0100, Peter N. Lundblad wrote:
> On Tue, 14 Feb 2006, Garrett Rooney wrote:
...
> I think the most famous example of this is our path manipulation functions
> that expect canonical paths (no double slashes except where required, no
> slash at the end). It would be both inefficent and cumbersome to have each
> such function check that the input is valid in this case.  Here, the input
> validation must be done on a higher level.

So, this might be a perfect example of a relatively unefficient, but
potentially worthy assertion in the above (ie original) sense.

-- 
Chris Stork   <>  Support eff.org!  <>   http://www.ics.uci.edu/~cstork/
OpenPGP fingerprint:  B08B 602C C806 C492 D069  021E 41F3 8C8D 50F9 CA2F

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Tue, 14 Feb 2006, Garrett Rooney wrote:

> On 2/14/06, Julian Foad <ju...@btopenworld.com> wrote:
>
> > Not true.  If one accepts that "assert" is allowed at all, it's perfectly valid
> > to check arguments of private functions because we know all the callers and
> > what they can do.  As for public APIs, that's partly what the debate is about,> and the answer is far from obvious.
>
> That's not exactly true either.  We've had (for as long as I can
> remember) a policy that we don't explicitly check for valid arguments
> to ANY API calls, if a caller passes us bogus data (i.e. a NULL
> pointer to a function defined to not take NULL pointers, etc) we
> crash, with the end result being that these sort of problems are
> usually found quickly and fixed.  There's no real difference between
> enforcing such things via an assert and enforcing them via the fact
> that we dereference a NULL pointer a few lines later.
>
I think the most famous example of this is our path manipulation functions
that expect canonical paths (no double slashes except where required, no
slash at the end). It would be both inefficent and cumbersome to have each
such function check that the input is valid in this case.  Here, the input
validation must be done on a higher level.

> The fact that in this particular case there was a bug that allowed
> such a situation to be triggered by a client, and that bug didn't get
> fixed instantly is unfortunate, but it doesn't make the general policy
> wrong.
>
Agreed.

//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Daniel Berlin <db...@dberlin.org>.
On Tue, 2006-02-14 at 10:14 -0800, Garrett Rooney wrote:
> On 2/14/06, Julian Foad <ju...@btopenworld.com> wrote:
> 
> > > That obviously means we can't use assert to check function arguments,
> >
> > Not true.  If one accepts that "assert" is allowed at all, it's perfectly valid
> > to check arguments of private functions because we know all the callers and
> > what they can do.  As for public APIs, that's partly what the debate is about,> and the answer is far from obvious.
> 
> That's not exactly true either.  We've had (for as long as I can
> remember) a policy that we don't explicitly check for valid arguments
> to ANY API calls, if a caller passes us bogus data (i.e. a NULL
> pointer to a function defined to not take NULL pointers, etc) we
> crash, with the end result being that these sort of problems are
> usually found quickly and fixed.  There's no real difference between
> enforcing such things via an assert and enforcing them via the fact
> that we dereference a NULL pointer a few lines later.

Dereferncing NULL doesn't crash on some platforms.  It will just give
you back NULL.
AIX is one of them, for example. 

Relying on this to crash is a bad idea.

(This is done for optimization purposes, it lets you move null checks
around without working that they will trap :P)



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 2/14/06, Julian Foad <ju...@btopenworld.com> wrote:

> I had a quick peek in the list archives just now, but certainly not a thorough
> search.  From what I see and what I remember of the last couple of times this
> came up, the discussions have always been the same as this one has been so far:
> one camp says "assert is good" and the other says "assert is evil" and nobody
> learns anything.
>
> I take this as a brush-off and a sign that either this discussion isn't welcome
> here, or I'm going about it the wrong way.  Being tactless, probably; that's
> what I'm good at.  If anyone thinks I should take this to a more appropriate
> forum, please say.
>
> Sorry for irritating you, Garrett.  I don't want to fight.  I really want to
> learn what the software world's best advice is on this matter, as much for my
> own education as for Subversion itself.

FWIW, I'm not referring to discussions about assert, I'm referring to
discussions about proactively checking for NULL arguments and things
like that, which we have discussed numerous times and the conclusion
has always been the status quo, we assume our API is used correctly
and if it isn't then we'll fail soon enough when we dereferrence a
NULL pointer.  I see this as essentially the same as the situation
with asserts.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: assert() vs. return(error)

Posted by Julian Foad <ju...@btopenworld.com>.
Garrett Rooney wrote:
> On 2/14/06, Julian Foad <ju...@btopenworld.com> wrote:
>>
>>That's at least the second time someone's said, "We've had a policy".  I would
>>say instead, "We've had a habit."  I haven't heard a good argument for this habit.
> 
> It's been discussed numerous times, see the mailing list archives.

I had a quick peek in the list archives just now, but certainly not a thorough 
search.  From what I see and what I remember of the last couple of times this 
came up, the discussions have always been the same as this one has been so far: 
one camp says "assert is good" and the other says "assert is evil" and nobody 
learns anything.

I take this as a brush-off and a sign that either this discussion isn't welcome 
here, or I'm going about it the wrong way.  Being tactless, probably; that's 
what I'm good at.  If anyone thinks I should take this to a more appropriate 
forum, please say.

Sorry for irritating you, Garrett.  I don't want to fight.  I really want to 
learn what the software world's best advice is on this matter, as much for my 
own education as for Subversion itself.


OK, here's maybe partly where I'm going wrong: posing the question as "assert 
versus error".  The real issue is not that clear-cut, and maybe that subject 
line tends to make people think the answer should be simply one or the other.

There are many dimensions to the issue.  Public or private interface; server or 
library or client program; checking parameters and/or result and/or internal 
consistency; pointer or other data type; etc.  The decision in each case can be 
to use assert or error or no check at all, and perhaps external testing (as in 
a test suite).  "The best" solution, or what we should aim for if we only knew 
what it was, will inevitably be a combination of techiques.

I haven't provided any answers, but hopefully the scope of the question is a 
little clearer.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 2/14/06, Julian Foad <ju...@btopenworld.com> wrote:
> Garrett Rooney wrote:
> > On 2/14/06, Julian Foad <ju...@btopenworld.com> wrote:
> >
> >>>That obviously means we can't use assert to check function arguments,
> >>
> >>Not true.  If one accepts that "assert" is allowed at all, it's perfectly valid
> >>to check arguments of private functions because we know all the callers and
> >>what they can do.  As for public APIs, that's partly what the debate is about,
>  >>and the answer is far from obvious.
> >
> > That's not exactly true either.  We've had (for as long as I can
> > remember) a policy that we don't explicitly check for valid arguments
>
> That's at least the second time someone's said, "We've had a policy".  I would
> say instead, "We've had a habit."  I haven't heard a good argument for this habit.

It's been discussed numerous times, see the mailing list archives.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: assert() vs. return(error)

Posted by Julian Foad <ju...@btopenworld.com>.
Garrett Rooney wrote:
> On 2/14/06, Julian Foad <ju...@btopenworld.com> wrote:
> 
>>>That obviously means we can't use assert to check function arguments,
>>
>>Not true.  If one accepts that "assert" is allowed at all, it's perfectly valid
>>to check arguments of private functions because we know all the callers and
>>what they can do.  As for public APIs, that's partly what the debate is about,
 >>and the answer is far from obvious.
> 
> That's not exactly true either.  We've had (for as long as I can
> remember) a policy that we don't explicitly check for valid arguments

That's at least the second time someone's said, "We've had a policy".  I would 
say instead, "We've had a habit."  I haven't heard a good argument for this habit.

> to ANY API calls, if a caller passes us bogus data (i.e. a NULL
> pointer to a function defined to not take NULL pointers, etc) we
> crash, with the end result being that these sort of problems are
> usually found quickly and fixed.  There's no real difference between
> enforcing such things via an assert and enforcing them via the fact
> that we dereference a NULL pointer a few lines later.

That's not exactly true either.  :-)  That's true only for invalid pointers 
(and possibly a few other things like division by zero), but not true for other 
parameter errors such as the one we encountered just now where two objects are 
supposed to match but don't.

I don't generally want to see assertions to catch null pointers, because the 
hardware does it for us much more efficiently.  That doesn't address the 
question of when it's good to use "assert" for other checks.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 2/14/06, Julian Foad <ju...@btopenworld.com> wrote:

> > That obviously means we can't use assert to check function arguments,
>
> Not true.  If one accepts that "assert" is allowed at all, it's perfectly valid
> to check arguments of private functions because we know all the callers and
> what they can do.  As for public APIs, that's partly what the debate is about,> and the answer is far from obvious.

That's not exactly true either.  We've had (for as long as I can
remember) a policy that we don't explicitly check for valid arguments
to ANY API calls, if a caller passes us bogus data (i.e. a NULL
pointer to a function defined to not take NULL pointers, etc) we
crash, with the end result being that these sort of problems are
usually found quickly and fixed.  There's no real difference between
enforcing such things via an assert and enforcing them via the fact
that we dereference a NULL pointer a few lines later.

Note that this policy isn't just ours, it's also used in other
prominent C programs like the Apache HTTP Server, libraries like APR,
etc.

The fact that in this particular case there was a bug that allowed
such a situation to be triggered by a client, and that bug didn't get
fixed instantly is unfortunate, but it doesn't make the general policy
wrong.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: assert() vs. return(error)

Posted by Daniel Berlin <db...@dberlin.org>.
On Tue, 2006-02-14 at 16:21 +0000, Julian Foad wrote:
> Daniel Berlin wrote:
> > On Tue, 2006-02-14 at 15:13 +0000, Julian Foad wrote:
> >>Stuart Celarier wrote:
> >>
> >>>The rest of the C world uses the assert macro to compile assertions in
> >>>to the debug version, and to remove assertions in the release version.
> [...]
> >>
> >>You're over-simplifying.
> > 
> > He's also wrong.
> > GCC, for example, specifically keeps asserts on in the release builds
> 
> Does it?  <searching...>  Aaargh!  I've just spent twenty minutes searching for 
> any documentation that says that GCC does that.  Now I realise you mean the GCC 
> developers keep asserts on when building GCC.

Yes.  All packaged gcc releases are built with asserts enabled.

We can't control whether people pass -DNDEBUG, etc, or not 
> 
> Yes, sure, some people do that.
> 
> - Julian
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Julian Foad <ju...@btopenworld.com>.
Daniel Berlin wrote:
> On Tue, 2006-02-14 at 15:13 +0000, Julian Foad wrote:
>>Stuart Celarier wrote:
>>
>>>The rest of the C world uses the assert macro to compile assertions in
>>>to the debug version, and to remove assertions in the release version.
[...]
>>
>>You're over-simplifying.
> 
> He's also wrong.
> GCC, for example, specifically keeps asserts on in the release builds

Does it?  <searching...>  Aaargh!  I've just spent twenty minutes searching for 
any documentation that says that GCC does that.  Now I realise you mean the GCC 
developers keep asserts on when building GCC.

Yes, sure, some people do that.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Daniel Berlin <db...@dberlin.org>.
On Tue, 2006-02-14 at 15:13 +0000, Julian Foad wrote:
> Stuart Celarier wrote:
> > The rest of the C world uses the assert macro to compile assertions in
> > to the debug version, and to remove assertions in the release version.
> > Subversion always compiles with assertions in the code, including the
> > release version.
> > 
> > For Subversion, assert means "conditional abort."
> > 
> > I haven't seen another software project change the meaning of assert in
> > this way, so I don't think much of the existing writing on assert
> > applies to Subversion.
> 
> You're over-simplifying.

He's also wrong.
GCC, for example, specifically keeps asserts on in the release builds

We do count as being part of the "rest of the C world".



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Branko Čibej <br...@xbc.nu>.
Julian Foad wrote:
> Branko Čibej wrote:
>> That obviously means we can't use assert to check function arguments,
>
> Not true.  If one accepts that "assert" is allowed at all, it's 
> perfectly valid to check arguments of private functions because we 
> know all the callers and what they can do.  As for public APIs, that's 
> partly what the debate is about, and the answer is far from obvious.
Ah yes, I meant "public API arguments".

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Julian Foad <ju...@btopenworld.com>.
Stuart Celarier wrote:
> The rest of the C world uses the assert macro to compile assertions in
> to the debug version, and to remove assertions in the release version.
> Subversion always compiles with assertions in the code, including the
> release version.
> 
> For Subversion, assert means "conditional abort."
> 
> I haven't seen another software project change the meaning of assert in
> this way, so I don't think much of the existing writing on assert
> applies to Subversion.

You're over-simplifying.  Don't read too much into anything that was said in a 
mailing list thread.  :-)  Subversion does not define assert in this way, it's 
just that some builds of Subversion do (like perhaps some or all of the 
official Windows binaries).  Anyway, the "goes away in release builds" aspect 
is not the only significant aspect discussed in writing.

We might well want to use assertions that remain in release mode.  We might 
well want to use both kinds.  We haven't discussed it enough yet.


Branko Čibej wrote:
> The "unwritten policy" about Subversion and assert is that we use assert 
> to check for conditions that "can't happen". 

I don't think most developers have a consistent view on that.  Mostly, asserts 
are simply not used, and I feel this is because many people fear them because 
they see only the obvious result that the program will terminate, and, lacking 
the knowledge and experience to see where this is a good thing, just assume 
that it is a bad thing.

> That obviously means we can't use assert to check function arguments,

Not true.  If one accepts that "assert" is allowed at all, it's perfectly valid 
to check arguments of private functions because we know all the callers and 
what they can do.  As for public APIs, that's partly what the debate is about, 
and the answer is far from obvious.

> and it's especially evil 
> if a malicious client can trigger an assert on the server.

Certainly.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: assert() vs. return(error)

Posted by Branko Čibej <br...@xbc.nu>.
Stuart Celarier wrote:
> All of the literature on assert (per se) seems to be mostly irrelevant
> because the Subversion code uses assert in a non-standard way [1].
>
> The rest of the C world uses the assert macro to compile assertions in
> to the debug version, and to remove assertions in the release version.
> Subversion always compiles with assertions in the code, including the
> release version.
>   
Ah hah. And that "rest of the world" also does not generate debug 
symbols for release mode builds, yes? We "violate" that "rule" too, luckily.

The "unwritten policy" about Subversion and assert is that we use assert 
to check for conditions that "can't happen". That obviously means we 
can't use assert to check function arguments, and it's especially evil 
if a malicious client can trigger an assert on the server.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org