This new test exercises every path where the user might possibly
pass in an invalid matrix. Currently the test fails if no error
is reported. Also, if an incorrect error is reported, (such as
CAIRO_STATUS_NO_MEMORY instead of CAIRO_STATUS_INVALID_MATRIX),
this is logged as a warning in invalid-matrix.log, but the test
still passes.
It would still be worthwhile to follow up quickly and fix those
cases to propagate the correct error value.
Also fix all callers to notice and propagate the error, (though
some paths will still lose the CAIRO_STATUS_INVALID_MATRIX value
due to a return value of NULL at one point).
Fixing this uncovered a leak of a CAIRO_INT_STATUS_UNSUPPORTED value
up to cairo_show_page, (and similarly to cairo_copy_page). There was
really no good reason for _cairo_surface_show_page and
_cairo_surface_copy_page to be returning cairo_int_status_t. Fix
this by simply handling the UNSUPPORTED return at the surface layer
instead of the gstate layer.
Obviously, the new name is _cairo_pdf_surface_emit_to_unicode_stream which
is consistent with the to_unicode_stream identifiers already existing in
the implementation.
This class of functions modify their argument and return it as a
*convenience* for the caller. For example, within cairo a common idiom is:
cairo_object_reference (new);
cairo_object_unreference (this->obj);
this->obj = new;
which updates the member to the new object irrespective of whether the
new object is the same as the one being replaced. Other issues arise
with subtypes, as the return type is the parent's and so require more
complicated handling to compile cleanly.
Disabling the warning is therefore preferred over adding code which
decreases readibility and reduces maintainability. We need to make the
compiler work for us, not against us...
As previously implemented, there's no essential information in the
return value from _cairo_dotostr, (the caller can simply use strlen
to recompute the same value, which is what the only caller is already
doing).
There would be real information in a return value which would return
the result from the call to snprintf for the case where the buffer is
not large enough for the number being printed.
Add a _cairo_traps_status function and use it instead of adding
error checks to callers of _cairo_traps_add_trap and
_cairo_traps_add_trap_from_points, (both of which are now given
a void return type).
Note: It looks like it would be convenient to shove a status value
into cairo_path_fixed_t to reduce this sequence of error checks.
But I tried that first, and it actually makes things worse overall
due to many things like _cairo_path_fixed_move_to called by
cairo_move_to where the result must be immediately checked anyway.
So I've already rejected that approach.
Now, the functions to add new data to a polygon all become void,
and there's a new _cairo_polygon_status call to query the status
at the end of a sequence of operations.
With this change, we fix many callerswhich were previously not
checking the return values of _cairo_polygon functions by adding
only a single call to _cairo_polygon_status rathern than several
new checks.
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.
