[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1222522 - /subversion/branches/1.7.x/STATUS

From: <schamel23_at_spinor.com>
Date: Sat, 24 Dec 2011 23:03:51 +0100

On 2011-12-24 16:58, Branko Čibej wrote:
> 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.

Why not just returning an error?

>
> -- Brane
>
Received on 2011-12-24 23:04:25 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.