Adrian Johnson pointed out a couple of mistakes in my previous attempt,
535e7c161b, to ensure propagation of errors
throughout the generation of the type1 subset.
This time the status member of the cairo_type1_font_subset_t is removed
in favour of simply return the error status from each function. This
completely avoids the issue of whether we overwrite a pre-existing error
status and confustion of status returns and the status member. The
removal of the status from the structure is possible due to its
short-lived nature - it is not exposed outside of the
_cairo_type1_subset_init() function, and is not shared by any other
piece of code.
Be careful not to overwrite existing the error status when propagating
errors and to not blindly return INT_STATUS_UNSUPPORTED from
load_truetype_table() as this will mask fatal errors.
Put a guard that checks the context's status at the start of each
getter that prevents the function from trying to dereference NULL state.
Use the status, as opposed to the invalid reference count, for
consistency with the existing guards on some of the getters.
The _cairo_ft_unscaled_font_fini() was skipped during the destroy for
fonts generated by cairo_ft_font_face_create_for_ft_face(). However
this causes a mutex to be 'leaked' for each font.
Every time we assign or return a hard-coded error status wrap that value
with a call to _cairo_error(). So the idiom becomes:
status = _cairo_error (CAIRO_STATUS_NO_MEMORY);
or
return _cairo_error (CAIRO_STATUS_INVALID_DASH);
This ensures that a breakpoint placed on _cairo_error() will trigger
immediately cairo detects the error.
As pointed out by Jeff Muizelaar, this allows for more concise code, as
_cairo_error(CAIRO_STATUS_NO_MEMORY)
return CAIRO_STATUS_NO_MEMORY
can become
return _cairo_error(CAIRO_STATUS_NO_MEMORY);
Since the objects can be shared and may be in use simultaneously across
multiple threads, setting the status needs to be atomic. We use a
locked compare and exchange in order to avoid overwriting an existing
error - that is we use an atomic operation that only sets the new status
value if the current value is CAIRO_STATUS_SUCCESS.
At some point during the blitz, I accidentally wrote
_cairo_error (CAIRO_STATUS_SUCCESS) and then proceeded to paste it into
the next 30 error sites! s/CAIRO_STATUS_SUCCESS/CAIRO_STATUS_NO_MEMORY/
As we now generate empty paths, we must be able to handle empty paths
in the user facing API. cairo_append_path() has an explicit check, and
raises an error, for a NULL path->data, so we need to check the
path->num_data first for empty paths.
pixman does not (yet?) support arbitrary strides and will fail to
generate an image for the data. We misinterpret that failure as a
CAIRO_STATUS_NO_MEMORY, so instead return CAIRO_STATUS_INVALID_FORMAT
before attempting to create the pixman image.
Avoid returning the "generic" _cairo_surface_nil (which corresponds to
CAIRO_STATUS_NO_MEMORY) when the user asks us to create a surface with
an invalid format or content.
Generate a real empty path structure instead of returning
_cairo_path_nil, if we have been asked to create an empty path.
(Also add a couple of missing _cairo_error()s and an appropriate test
case.)
Spotted by Fred Kiefer.
libtool causes the .gcda files to be generated in the .libs/ directory,
separate from the .c source file. lcov expects them to be in the same
directory - so massage the lcov.info file to remove the reference to the
.libs/.
Also separate the target for generating the lcov output, so that it can
be run independently from triggering the tests. And improve convenience
of using the other lcov targets.