Re: problem revealed by issue #2398 (server-side assertion)

"Stuart Celarier" <SCelarier@corillian.com> writes:
> Karl wrote that there is an official policy on using assert in the
> Subversion code, but that it "is so common-sensical that we don't bother
> to write it down." [1]
>
> Can you help me understand what the official policy is? Maybe more than
> one thing can make common sense.

Maybe we should write this in hacking.html, hmm.

The policy is simply that Subversion should never fail an assert due
to data under user control, when Subversion has no way to know that
the data is invalid at some point before the assert. In other
words, no user-triggerable assertion failures.

According to the above policy, it would be okay to leave the checks as
asserts. (Of course, they need to assert the correct thing, which
they weren't before, but we know how to fix that now.)

However, just because we *can* use an assert doesn't mean we must. In
this case, Subversion could have detected the error condition before
the assert, by putting better checks in the callers of the function(s)
containing the assert, but did not. I'm not sure where to put them
yet, and felt that an error return would be more useful in helping us
track down the call sites, while not ticking off server admins :-).

I understand Garrett's argument that using assert for a "can't happen"
condition is best, but in practice that didn't seem to make us solve
the bug faster, did it? In the meantime, people's servers were
crashing...

-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org