[cairo-commit] 2 commits - src/cairo.c test/nil-surface.c

[Carl Worth]

 src/cairo.c        |   10 +++++++++-
 test/nil-surface.c |   23 +++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

New commits:
diff-tree 6da7f140334835be9a972db75de78d99b8bd24b1 (from 36590fd4702cc24acacd20e4394c902e44be46c6)
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Mar 2 03:49:11 2007 -0800

    Fix INVALID_RESTORE case to avoid crashes
    
    Previously, an INVALID_RESTORE error would leave cr->gstate as NULL,
    (which is generally impossible/invalid). This seems safe enough as
    most cairo functions check cr->status first and bail if anything
    looks fishy.
    
    However, the many cairo_get functions happily march along in spite
    of any current error. We could instrument all of those functions to
    check for the error status and return some dummy value in that case.
    But it's much easier to get the same basic effect by simply creating
    a non-NULL cr->gstate which will hold all those dummy values, and
    we can eliminate the crashes without having to touch up every
    cairo_get function.
    
    This fixes the bug reported here:
    
    	evolution crash to _cairo_gstate_backend_to_user()
    	https://bugs.freedesktop.org/show_bug.cgi?id=9906
    
    It also eliminates the crash that was added to the nil-surface test
    with the previous commit.

diff --git a/src/cairo.c b/src/cairo.c
index b487bab..fb799e1 100644
--- a/src/cairo.c
+++ b/src/cairo.c
@@ -408,8 +408,16 @@ cairo_restore (cairo_t *cr)
 
     _cairo_gstate_destroy (top);
 
-    if (cr->gstate == NULL)
+    if (cr->gstate == NULL) {
 	_cairo_set_error (cr, CAIRO_STATUS_INVALID_RESTORE);
+	/* We go ahead and create a new gstate here, just for the sake
+	 * of the various cairo_get functions that don't check
+	 * cr->status. This gstate should never be written to, (since
+	 * the cairo functions that do modify anything all check
+	 * status and immediately abort).
+	 */
+	cr->gstate = _cairo_gstate_create ((cairo_surface_t *)&_cairo_surface_nil);
+    }
 }
 slim_hidden_def(cairo_restore);
 
diff-tree 36590fd4702cc24acacd20e4394c902e44be46c6 (from 712447856dc5cf559fcdbceaf902a39fd5eddef9)
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Mar 2 03:43:46 2007 -0800

    Add test of cairo_get_* after INVALID_RESTORE to nil-surface
    
    This new test demonstrates a crash condition as reported here:
    
    	evolution crash to _cairo_gstate_backend_to_user()
    	https://bugs.freedesktop.org/show_bug.cgi?id=9906

diff --git a/test/nil-surface.c b/test/nil-surface.c
index 3ae7126..c672e64 100644
--- a/test/nil-surface.c
+++ b/test/nil-surface.c
@@ -30,6 +30,7 @@
  *
  *	https://bugs.freedesktop.org/show_bug.cgi?id=4088
  *	https://bugs.freedesktop.org/show_bug.cgi?id=3915
+ *	https://bugs.freedesktop.org/show_bug.cgi?id=9906
  */
 
 static cairo_test_draw_function_t draw;
@@ -113,6 +114,28 @@ draw (cairo_t *cr, int width, int height
     cairo_surface_finish (surface);
     cairo_surface_destroy (surface);
 
+    /*
+     * 4. OK, we're straying from the original name, but it's still a
+     * similar kind of testing of error paths. Here we're making sure
+     * we can still call a cairo_get_* function after triggering an
+     * INVALID_RESTORE error.
+     */
+    cr2 = cairo_create (cairo_get_target (cr));
+
+    /* Trigger invalid restore. */
+    cairo_restore (cr2);
+    if (cairo_status (cr2) != CAIRO_STATUS_INVALID_RESTORE) {
+	cairo_test_log ("Error: Received status of \"%s\" rather than expected \"%s\"\n",
+			cairo_status_to_string (cairo_status (cr2)),
+			cairo_status_to_string (CAIRO_STATUS_INVALID_RESTORE));
+	return CAIRO_TEST_FAILURE;
+    }
+
+    /* Test that we can still call cairo_get_fill_rule without crashing. */
+    cairo_get_fill_rule (cr2);
+
+    cairo_destroy (cr2);
+
     return CAIRO_TEST_SUCCESS;
 }