[cairo] [PATCH] Avoid returning NULL from cairo_pop_group()

[Behdad Esfahbod]

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