[cairo] How soon to call cairo_error?

[Behdad Esfahbod]

On Fri, 2007-10-26 at 20:48 -0400, Carl Worth wrote:
> 
> 1) Cairo itself might handle the non-success status, so the
>    lowest-level assignment might not actually be an error at all, (so
>    reporting it with _cairo_error is just plain wrong).

Right.  This was my main concern too.  However, I also knew that there
are a lot of places we are not correctly calling _cairo_error().  But I
knew it's not an easy task.  When I saw Chris do it, I assumed he has
done his homework, and I'm happier fixing any introduced bugs than going
to the previous state.

Incidentally I implemented a similar scheme in HarfBuzz recently, and I
made a simple rule for myself: all errors other than HB_Err_Ok and
HB_Err_Not_Covered should call _hb_err() on the site.  Some
HB_Err_Not_Covered errors can also call _hb_err() but no HB_Err_Ok ever.
Setting those rules, it was obvious to automatically add the annotation
over the 15000 lines of code I had.

I suggest we go in that direction in cairo.  That is, all errors other
than SUCCESS and UNSUPPORTED should be actual errors that are not worked
around.  In the case of the fixup commit you pointed to, it means that
_cairo_path_fixed_get_current_point() should be changed to return a
cairo_bool_t of whether there is a current point, instead of an abused
cairo_status_t that can only be SUCCESS or NO_CURRENT_POINT.


> 2) We previously had tried to call _cairo_error as high as possible
>    within cairo's stack, (as close to the user code as possible), so
>    this change may introduce multiple calls to _cairo_error for a
>    single error. That could be distracting.

Possibly.  But if we only call it when the error is originated, and the
rest of the code correctly protects against already-in-error situations,
there shouldn't be multiple _cairo_error calls per user call.


> 3) The deeper calls to _cairo_error mean that the user will be plunged
>    deeper into cairo's guts when debugging[*]. That could be
> annoying. 

Possibly.  But then again, the user need simply look at the cairo
entrance point.  For debugging cairo bugs on the other hand, it's
extremely useful to see where the error is coming from.  Since most
cairo bugs are discovered by users hitting it, it's as important to be
able to get such deep traces from users upon request.


Anyway, that was my thoughts on the issue.

Cheers,

-- 
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