The cairo_image_surface_create function never returns NULL so
the previous error checks would never have triggered. The correct
approach is to check the result of cairo_surface_status().
This is just multiplication after all, so there's nothing that can fail.
And we can get rid of a lot of useless error-checking code this way.
The corrected functions are:
_cairo_gstate_user_to_device
_cairo_gstate_user_to_device_distance
_cairo_gstate_device_to_user
_cairo_gstate_device_to_user_distance
Now that we have the warn_unused_result attribute enabled, (thanks
Chris!), it's actually harmful to have a function return an
uncoditional value of CAIRO_STATUS_SUCCESS. The harm is that
it would force lots of unnecessary error-checking paths that
just add clutter.
It is much better to simply give a function that cannot fail
a return type of void.
A few tests were failing due to clip_init_deep_copy() not being able to
clone the target surface. Before propagating the failure, this was being
silently ignored.
Copy the simple implementation from cairo-image-surface.
Currently if the ownership of the bitmap->buffer is passed to
_get_bitmap_surface() then the status of the buffer is inconsistent
should the function detect an error (i.e. CAIRO_STATUS_NO_MEMORY).
Fix it up that should we encounter an error and we own the buffer then
we always free it on behalf of the caller.
Confusion had been introduced as to who provided the fixup after
the malloc failed which resulted in a NULL deference whilst checking for
an erroneous pattern in _cairo_pattern_create_in_error.
Currently the code defaults to setting its points to NULL and fixing it up
on the first add_point() to use the embedded buffer. Skip this extra step
by initialising points to the embedded buffer.
The idiom for cairo.c is to do
cr->status = _cairo_op ();
if (cr->status) _cairo_set_error (cr, cr->status);
Unfortunately a trivial mistake for a _cairo_op () is to call a cairo_op ()
and forget to check cr->status but return CAIRO_STATUS_SUCCESS which will
mask the earlier error.
Obviously this is a bug in the lower level but the impact can be reduced
by chaning cairo.c to use a local status variable for its return:
cairo_status_t status = _cairo_op ();
if (status) _cairo_set_error (cr, cr->status);
These were found during a cairo_static pass on an alternative branch...
A critical one in particular was setting the have added glyph flag to
TRUE even if _cairo_xlib_surface_add_glyph() fails. This can cause an
application crash due to a RenderBadGlyph error later when the scaled
font is cleaned and we attempt to remove the glyph.
_cairo_pattern_release_surface() asserts that it is passed a pattern
surface. This itself is bad as breaks the symmetry with
_cairo_pattern_acquire_surface under() error conditions, however reorder
the cleanup to avoid this assertion.
The text perf-case tries to fill the region with a single text string,
but fails to detect when the current point does not advance due to an
error. This causes the perf-case to enter an infinite loop, so we break
out when the cairo_status() has been set.
Detect when a substitute image surface is returned for a solid pattern,
and avoid mixed image/xlib composite operations. This can happen for example
if there is a resource allocation failure during creating a similar surface.
Add status returns to functions in order to propagate an error up
the call stack.
For the emit_*_pattern we add a new status return even when when
the functon return CAIRO_STATUS_SUCCESS unconditionally in order for
the caller to handle all cases in a consistent manner.
