[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: Branko Čibej <brane_at_xbc.nu>
Date: Sun, 25 Dec 2011 06:42:08 +0100

On 24.12.2011 23:03, schamel23_at_spinor.com wrote:
> 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?

You're confusing the issue. I was responding to Stefan's assertion (sic)
that asserts (or rather, the aborts they cause) should be disabled in
production builds, and giving an example of what happens if you do that.

Having been down that road myself, I naturally assume Stefan's
assumption stems from the fact that Microsoft's compilers (and, once
upon a time, even libraries) do that by default. Microsoft even
recommends that practice for writing kernel device drivers ...
presumably on the assumption that it's better to trash your data than
crash your system?

-- Brane
Received on 2011-12-25 06:42:47 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.