[cairo] [PATCH] Avoid returning NULL from cairo_pop_group()
Behdad Esfahbod
behdad at behdad.org
Tue Sep 11 22:31:01 PDT 2007
On Tue, 2007-09-11 at 20:28 -0400, Jeff Muizelaar wrote:
> It looks cairo_pop_group can theoretically return NULL.
Yep, good catch. Feel free to push.
behdad
> cairo_pattern_t *
> cairo_pop_group (cairo_t *cr)
> {
> cairo_surface_t *group_surface, *parent_target;
> cairo_pattern_t *group_pattern = NULL;
> cairo_matrix_t group_matrix;
>
> if (cr->status)
> return (cairo_pattern_t*) &_cairo_pattern_nil.base;
>
> /* Grab the active surfaces */
> group_surface = _cairo_gstate_get_target (cr->gstate);
> parent_target = _cairo_gstate_get_parent_target (cr->gstate);
>
> /* Verify that we are at the right nesting level */
> if (parent_target == NULL) {
> _cairo_set_error (cr, CAIRO_STATUS_INVALID_POP_GROUP);
> return (cairo_pattern_t*) &_cairo_pattern_nil.base;
> }
>
> /* We need to save group_surface before we restore; we don't need
> * to reference parent_target and original_target, since the
> * gstate will still hold refs to them once we restore. */
> group_surface = cairo_surface_reference (group_surface);
>
> cairo_restore (cr);
>
> if (cr->status)
> goto done;
> group_pattern is still NULL at this point
>
> group_pattern = cairo_pattern_create_for_surface (group_surface);
> if (cairo_pattern_status (group_pattern)) {
> _cairo_set_error (cr, cairo_pattern_status (group_pattern));
> goto done;
> }
>
> _cairo_gstate_get_matrix (cr->gstate, &group_matrix);
> cairo_pattern_set_matrix (group_pattern, &group_matrix);
> done:
> cairo_surface_destroy (group_surface);
>
> return group_pattern;
> }
>
> This patch should fix it, and in my opinion makes the code better.
> There isn't any use, as far as I can see, for the NULL group_pattern.
>
>
> diff --git a/src/cairo.c b/src/cairo.c
> index 371a343..ff052b5 100644
> --- a/src/cairo.c
> +++ b/src/cairo.c
> @@ -549,11 +549,11 @@ cairo_pattern_t *
> cairo_pop_group (cairo_t *cr)
> {
> cairo_surface_t *group_surface, *parent_target;
> - cairo_pattern_t *group_pattern = NULL;
> + cairo_pattern_t *group_pattern = (cairo_pattern_t*) &_cairo_pattern_nil.base;
> cairo_matrix_t group_matrix;
>
> if (cr->status)
> - return (cairo_pattern_t*) &_cairo_pattern_nil.base;
> + return group_pattern;
>
> /* Grab the active surfaces */
> group_surface = _cairo_gstate_get_target (cr->gstate);
> @@ -562,7 +562,7 @@ cairo_pop_group (cairo_t *cr)
> /* Verify that we are at the right nesting level */
> if (parent_target == NULL) {
> _cairo_set_error (cr, CAIRO_STATUS_INVALID_POP_GROUP);
> - return (cairo_pattern_t*) &_cairo_pattern_nil.base;
> + return group_pattern;
> }
>
> /* We need to save group_surface before we restore; we don't need
> _______________________________________________
> cairo mailing list
> cairo at cairographics.org
> http://lists.cairographics.org/mailman/listinfo/cairo
--
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
More information about the cairo
mailing list