There's a difference between assert checks in the public API versus asserts in
the internal code. It's one thing not to assert that, say
svn_path_is_canonical() got a non-NULL path argument, but asserting deep in some
of our libraries is a different issue.
I don't think the suggestion is to have the public API now return svn_error_t's
all over the place, but if we have some internal function that asserts, it's
better for it to throw an error.
Blair
Branko Čibej wrote:
> I disagree with this idea. Very very much in fact. It's contrary to the
> API implementation rules we set up from day one -- one of which says
> that our API will not validate parameters. Also it would mean that all
> functions that use assert but happen not to return an svn_eror_t would
> have to be revved.
>
> The above implies that
>
> * assert should only be used for "this can't happen" sanity checks
> * it should be turned off in production code (which is what we do on
> Windows, IIRC).
>
> And of course you /can/ catch an assert in a debugger.
>
> -- Brane
>
> Julian Foad wrote:
>> PROBLEM
>> -------
>>
>> The standard C assertion, typically used at the beginning of a function
>> in statements like
>>
>> assert(arg1 != NULL);
>>
>> is an extremely useful self-checking tool for catching bugs, but in a C
>> library it has the unfortunate property that the program using the
>> library can't trap it. When that program is something big like an IDE or
>> the Windows desktop, the result of the ensuing crash is too destructive.
>>
>> Subversion's standard error reporting mechanism, which is mainly used
>> for reporting logical exceptions, can also be used to report bugs:
>>
>> if (! arg1)
>> return svn_error_create
>> (SVN_ERR_INCORRECT_PARAMS, NULL,
>> _("foo() cannot be called with a NULL arg1"));
>>
>> However, when written like this, it causes unnecessarily a decrease in
>> readability (4 lines just to indicate one simple fact) and an increase
>> in translator effort.
>>
>>
>> SOLUTION
>> --------
>>
>> A new macro:
>>
>> SVN_ERR_ASSERT(arg1 != NULL);
>>
>> which expands to something suitable, like the above code snippet,
>> returning an svn_error_t if the assertion fails.
>>
>>
>> Why does this seem too simple to be true?
>>
>> Any volunteers to do it?
>>
>> (Here's an improvement that can be made later: tweak the
>> "svn_error_clear" functions so that "assertion" errors can always be
>> discovered even if the programmer carelessly "clears" the error. Even
>> without this improvement, I think the new form will do a lot of good.)
>>
>> - Julian
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-13 18:49:03 CEST