[cairo] error handling questions

[Carl Worth]

On Sun, 01 Jul 2007 15:47:18 -0400, Behdad Esfahbod wrote:
> On Mon, 2007-06-18 at 16:52 -0400, Vladimir Vukicevic wrote:
> > We've been using CAIRO_STATUS_NO_MEMORY as "unknown error" in a bunch of
> > places; I know that it's generally been the policy to use a real error
> > code, but would anyone mind if I were to introduce
> > CAIRO_STATUS_UNKNOWN_ERROR (or CAIRO_STATUS_FAILURE) so that we can
> > distinguish between real OOM and unknown errors?
>
> I'm in favor.

Abusing NO_MEMORY was bad, inexcusable, and must be fixed. But adding
UNKNOWN_ERROR is not any better, (and worse in that it actually
_looks_ like it's intended as a random catch-all).

I don't want random catch-all error status values. If we wanted that,
we could just reduce the enum to a Boolean and be done with it.

I want the error status values to contain as much information as
reasonably possible. If the problem is that there's some backend that
doesn't provide suitably detailed error information, then that's what
the cairo error status should reflect, (CAIRO_STATUS_GDI_ERROR or
whatever).

> Any place that an error is initiated should call _cairo_error().  it's
> not like that for many error paths right now and those should be fixed.
> We don't have a strict rule though, I'm sure there are cases that
> _cairo_error should not be called (for UNSUPPORTED only?).  Carl may
> know how this is supposed to work,

If an error is getting lodged into an object or returned to the user,
then _cairo_error must be called, (and ideally just once). It
shouldn't ever be called at any other time.

> but I agree that our error handling is still a bit messy.

Yes, definitely. Chris recently, (or not so recently... I'm horribly
behind on things), pulled together a document describing the
error-handling strategy culled from a discussion between Havoc on
myself. I think that's useful and I'm hoping to get the cairo wiki
re-enabled _very_ shortly so it will have a place to live, (and so it
can get incorporated into cairo's documentation).

But what would be even more useful would be to go over that discussion
again and pull out the things that cairo's current error-handling
implementation is missing. What makes it hard to track down and fix
problems right now?

> Also, feel free to make _cairo_error() do something if CAIRO_DEBUG env
> var is set.  Options are raising SIGTRAP and printing a backtrace.

And that's probably a big part of the answer, (especially the
backtrace printing). I think a lot of what Havoc (and others) had to
complain about was failure without cairo giving more information,
(timely and detailed), about the problem.

> builds and it has been very handy.  The only thing I don't like here is
> that to produce useful backtraces we need to use Jeff's
> backtrace_symbols().

What's the concern here? Is it a lot of code to bloat the library?
Less portable than would desired? Something else?

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://cairographics.org/archives/cairo/attachments/20070703/abbfb0cf/attachment.pgp