Subject: Re: Patch: Error handling
From: Andrew Dunbar (hippietrail@yahoo.com)
Date: Mon Jun 18 2001 - 08:09:28 CDT
Joaquín Cuenca Abela wrote:
>
> 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.
I did try to check what the consequences were. I started with
the ones that were going to cause us to call pDialog->runModal() from
a NULL pDialog. This is going to happen on low memory, and when
dialogs are not yet implemented - not fun for users running release
builds. I left the asserts there as well so they wouldn't
just fail silently for us developers in the case of unimplemented
dialogs. I returned false on all of these because they were all
boolean functions - does ap_EditMethods need an error field?
Looks like I really goofed on the other pointers though ):
-- http://linguaphile.sourceforge.net _________________________________________________________ Do You Yahoo!? Get your free @yahoo.com address at http://mail.yahoo.com
This archive was generated by hypermail 2b25 : Mon Jun 18 2001 - 08:07:36 CDT