On Sat, Dec 24, 2011 at 11:24:14AM +0100, Branko Čibej wrote:
> On 24.12.2011 09:41, Stefan Küng wrote:
> > On 23.12.2011 23:52, Branko Čibej wrote:
> >> Ranting is all very well, but I've yet to hear a suggestion from you
> >> about how the libraries should handle unrecoverable errors. Like, for
> >> example, the case where wc.db contains inconsistent and/or invalid data.
> >
> > Simple: return an error!
> > That way the application can go on running, only the svn command is
> > stopped.
> > Sure, returning an error isn't always easy because it requires that
> > you code a path to return to a known state. But for a library that's
> > what has to be done.
>
> In the case of Subversion, that would imply:
>
> * second, removing all uses of SVN_ERR_MALFUNCTION* and SVN_ERR_ASSERT*;
I disagree. I am still putting assertions into the code at spots where
they are useful to catch bugs that get introduced by future code changes.
Just recently I added assertions to the hotcopy code to check variables
we rely on having certain values. That's totally fine. I made our test
suite exercise these assertions to make sure they don't trigger when
a hotcopy is made.
The problem we're having is that early in wc-ng development people
put a lot of assertions into code reading data from the DB tables.
Those assertions were put there with all the right intentions but they
aren't covered by the test suite and it turns out sometimes data can get
into the DB that triggers these assertions. This is improper use of
assertions.
Putting the Explorer integration issue aside, these assertions also
cause problems for users of the 'svn' client and for ourselves when
trying to diagnose issues people are running into. Assertions don't
provide any context information. People report these assertions to the
list and we have no idea why they were triggered because we lack
information a proper error message would have provided.
E.g. we seriously had to ask users to dig out of the DB on our behalf
the path to a file lacking its BASE checksum which triggered a
checksum != NULL assertion. A proper error includes this information.
This is needless work we're asking our users to do.
> * but first, making every library function return an svn_error_t.
I doubt that's really an issue.
Virtually any important call TSVN needs does return svn_error_t already.
Received on 2011-12-24 11:54:18 CET