Subject: Re: Patch: Error handling
From: Joaquín Cuenca Abela (cuenca@pacaterie.u-psud.fr)
Date: Mon Jun 18 2001 - 03:23:51 CDT
On 18 Jun 2001 16:30:42 +1000, Andrew Dunbar wrote:
> Just a few places we have ASSERTs on possible null pointers but were
> not doing anything about them. These were all places we could simply
> return false. Let me know if I'm out of line here (:
Andrew, you should take in account that the semantics of assert(expr)
are different from these of
if (!expr)
return error;
with "assert(expr)" we want to say "if expr is false, then we have an
error in the logic of the program". With "if (!expr) return error;"
we say "if expr is false, then we want to signal an error" (for
instance,
if we run out of disk space, and we try to save a file, we should
signal an error, but should not assert, because there is no error in
the logic of the program).
That said, some of the assert's that you change in your patch, should
be really in the form of:
if (!expr)
return error;
instead of assert(expr) (for instance, if run out of memory, and we
try to create a new dialog, it will be NULL, and we should try to manage
this case, instead of just assert'ing). So I will be happy if we apply
some of your changes, but please, be aware of the differences between
assert(expr) and if (!expr).
For instance:
UT_ASSERT(pFrame);
if (!pFrame)
return false;
has no sense, because if we UT_ASSERT(pFrame) we're saying "It's
IMPOSSIBLE to have pFrame != 0 (except if we have a bug in our code)",
and latter, we're catching the case when pFrame is != 0.
So overall, your patch looks good, just remove the old UT_ASSERT when
you use if (!expr), and double-check that you are fixing the right
asserts.
Keep the good work!
Cheers,
-- Joaquín Cuenca Abela cuenca@celium.net
This archive was generated by hypermail 2b25 : Mon Jun 18 2001 - 03:23:45 CDT