On 24.12.2011 11:57, Stefan Küng wrote:
> On 24.12.2011 11:53, Stefan Sperling wrote:
>> 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.
>
> asserts are not bad per se. They're very useful in debug builds. But
> for release builds they have to be replaced by something that returns
> an error message.
> That's why the c-runtime assert() is changed to an empty statement
> when NDEBUG is defined for release builds.
This is where I most strenuously disagree. Microsoft's insistence on
disabling asserts in what they call "production" builds is the stupidest
idea ever.
You put an assert into the code at a point where there is no safe way to
continue execution, where "safe" is defined as "will provably not
corrupt anything." Having asserts active only in "debug" builds assumes
that you have a test suite that provides 100% coverage of all possible
error conditions and will let you find all the bugs that can trigger the
assertion before you ship your production build. That assumption is
patently absurd.
So, in real life, you run your tests, find some bugs, decide that's good
enough, turn off assertions, install the production build somewhere,
encounter a bug that would have triggered the assertion (except the
assert is not there any more) and happily, silently zap your
database/disk/whatever. You'll have a hard time convincing me that this
is a better idea than crashing Explorer.
-- Brane
Received on 2011-12-24 16:58:52 CET