[cairo] error handling

[Behdad Esfahbod]

On Wed, 2007-05-30 at 11:07 +0100, Baz wrote:
> On 30/05/07, Behdad Esfahbod <behdad at behdad.org> wrote:
> > Quickly checked a couple of them.  Seems like in
> > cairo_scaled_font_glyph_extents() and probably others, you return any
> > error that happens.  I don't think that quite works.  For example, if an
> > out of memory error happens, we want to still keep the error in the
> > object.  Only the "ignorable" errors should be returned immediately.  In
> > this case, out-of-range glyph id is the only such error.
> 
> Ok, I was going with the do-one-or-the-other behaviour elsewhere in
> the code, and because I think there are more recoverable conditions
> than that one - as in, you may be able to keep drawing but the output
> is not exactly what you asked for,

No, if the output that the user asked for isn't generated it's an error.
Otherwise you could ignore all errors...

>  same as in the missing glyph case.

Not exactly.  Here is the reason: retained object status is for errors
that the user can avoid and for unavoidable errors.  Setting a
degenerate matrix on a cairo_t can be avoided by the user, so it puts
cairo_t in error state.  Ou-of-memory cannot be avoided, so it puts
cairo_t in error state.

Errors caused by missing glyphs in a call like cairo_show_text("Hello")
cannot be avoided by user: he just doesn't know if the font has glyphs
for "Hello" or not, and it's not unavoidable: the user can choose
another font for example.  So what makes most sense to me is to let the
user know what happened and let him decide.

> And, OOM errors are not kept for set_user_data? But I'm happy to
> change the patches.

Yes, that's a special case.  user_data is not really part of the cairo
drawing API but just object management facilities.  There's also another
reason: cairo_*set_user_data() has to return a status no matter what,
this is to let user manage lifecycle of the user data correctly even in
the case of failure in set_user_data().  And there is a rule that any
error should either be retained only or returned to user but not both.


> So do you want this:
>     if (status && status != CAIRO_STATUS_INVALID_GLYPH)
>        _cairo_set_error(status);
>     return status;
> 
> or this:
>     if (status == CAIRO_STATUS_INVALID_GLYPH)
>        return status;
>     if (status)
>        _cairo_set_error(status);
>     return CAIRO_STATUS_SUCCESS;

Definitely not the second.  Why is it returning SUCCESS.

> I can see justifications for both. In the first case, you don't need
> to check two different statuses to see if anything went wrong; in the
> second case warnings and errors don't get mixed together. Which would
> you prefer?

I think you get the difference now.  In short, only
CAIRO_STATUS_INVALID_GLYPH may be returned to user (or a better named
one for cairo_show_text()), and all other errors handled like are
currently done.

> > I don't see why you changed cairo_set_font_face and
> > cairo_select_font_face at all.
> 
> I was putting it in the category of 'bad user input that cairo can
> recover from'. The behaviour of what happens if you select a
> non-existent face isn't documented, and isn't programmer error. But, I
> guess this is better fixed by fixing the documentation to describe
> that it's just a best-effort font you get. Dropped.
> 
> I think doing set_font_face was just daftness on my part. It can only
> ever return programmer errors.
> 
> -Baz
-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759