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

Jeff Muizelaar jeff at infidigm.net
Tue Sep 11 17:28:09 PDT 2007


It looks cairo_pop_group can theoretically return NULL.

    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


More information about the cairo mailing list