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

Re: [PATCH] Replacement for "assert" in the libraries

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 19 Jun 2008 14:38:44 +0100

On Wed, 2008-06-18 at 10:55 -0700, David Glasser wrote:
> > +/** A type of function that handles an assertion failure or other internal
> > + * malfunction detected within the Subversion libraries.
[...]
> > + * A function of this type must do one of:
> > + * - Return an error object describing the error.
> > + * - Allow the program to continue, by returning SVN_NO_ERROR.
>
> What does that mean exactly? I mean, if I'm writing a function, and I
> write an assertion, then I assume that this means that if the
> assertion is false then the code's not going to keep running. I mean,
> if I read:
>
> SVN_ERR_ASSERT(pointer != NULL);
>
> something(*pointer);
>
> then I'm going to assume that, in fact, pointer is not NULL in the
> second line. Or
>
> SVN_ERR_ASSERT(there_are_no_secrets_in(some_structure));
>
> send_to_some_random_person_over_the_network(some_structure);
>
> that I'm never sharing secrets. But in your API, it's perfectly
> acceptable for somebody using our libraries to implement a malfunction
> handler which ignores the assertion or abort entirely? I think that
> sort of makes them worthless...

I don't understand why you would think that the ability to ignore an
assertion should "sort of make them worthless". Assertions aren't
security-enforcement tools, they're debugging tools.

The ability for some builds to do so unconditionally gives a similar
effect to compiling out traditional assertions in "release mode",
without the speed advantage but with the advantage that the handler can
log the fact that an assertion failed so that the information is not
lost.

The ability to write a callback function that selectively ignores
certain assertion failures that the user or the programmer has found to
be harmless (or indeed wrong) can be valuable. This facility can enable
users to get work done despite some trivial bug that gets triggered but
that doesn't affect the correctness of what they're trying to do.

Remember that the vast majority of assertions don't check "Am I about to
divulge a secret?" but trivial things like "Is this filename (that I'm
about to print, let's say) composed of legal filename characters?" In
these noraml cases, the developer needs to know that something has gone
wrong somewhere, but there are no disastrous consequences for the user
and the harm of terminating the program every time it hits this
assertion is likely to be far greater than the harm of allowing it to
proceed with printing this malformed filename.

> (And sure, you can compile out assert()s normally too,

Thereby nullifying your objection as you described it, surely?

> but not abort()s.)

That's true but the aborts I'm aiming at are just disguised assertions.
We can still leave real aborts in the code where we deem it necessary.

> > +svn_error_abort_on_malfunc(const char *file, int line, const char *expr);
>
> Maybe we want to add an environment variable or something that our
> command-line clients check and use this handler if it's set?

Maybe.

Thanks for the feedback.

- Julian

---------------------------------------------------------------------
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-19 15:39:25 CEST

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.