[cairo] [PATCH] xcb: Fix error reporting if fallback fails

Uli Schlachter psychon at znc.in
Tue May 30 16:24:22 UTC 2017


If we cannot let the X11 server do some operation (for example: the
RENDER extension is not available), then we fall back to an image
surface and do the operation locally instead. This fallback requires the
current content of the surface to be downloaded from the X11 server.
This fallback logic had an error.

The fallback is implemented with _get_image() in the function
_cairo_xcb_surface_fallback(). _get_image() is only called if we do not
yet have a fallback available, so when we call _get_image we have
surface->fallback == NULL. Then, if _get_image() fails, it returns a
surface in an error state.

Before this patch, the code would then just ignore this error surface
and return &surface->fallback->base, a NULL pointer. This would then
quickly cause a crash when e.g. the surface's ->status member is
accessed.

Fix this by returning the error surface instead as the fallback.

The end result of this patch will be that the XCB surface that is
currently drawn to ends up in an error state which is a lot better than
a NULL pointer dereference and actually correct in this case. The error
state is reached because the current drawing operation will fail and
this error is reported up the call stack and eventually "taints" the
surface.

(However, the error code could be better: _get_image() too often fails
with a generic CAIRO_STATUS_NO_MEMORY error, but that's left as future
work)

Signed-off-by: Uli Schlachter <psychon at znc.in>
---

[No test for this since in the case were I ran into this, _get_image()
failed because the X11 server closed the connection. No idea why it
did that.]

 src/cairo-xcb-surface.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/cairo-xcb-surface.c b/src/cairo-xcb-surface.c
index b319b7f24..39127c148 100644
--- a/src/cairo-xcb-surface.c
+++ b/src/cairo-xcb-surface.c
@@ -833,12 +833,13 @@ _cairo_xcb_surface_fallback (cairo_xcb_surface_t *surface,
     image = (cairo_image_surface_t *)
 	    _get_image (surface, TRUE, 0, 0, surface->width, surface->height);
 
+    if (image->base.status != CAIRO_STATUS_SUCCESS)
+	return &image->base;
+
     /* If there was a deferred clear, _get_image applied it */
-    if (image->base.status == CAIRO_STATUS_SUCCESS) {
-	surface->deferred_clear = FALSE;
+    surface->deferred_clear = FALSE;
 
-	surface->fallback = image;
-    }
+    surface->fallback = image;
 
     return &surface->fallback->base;
 }
-- 
2.11.0


More information about the cairo mailing list