[PATCH v2] xnest: Ignore GetImage() error in xnestGetImage()

[Jamey Sharp]

I believe you don't want to call XSync after XGetImage returns, since by
the time that call returns you're guaranteed that any error it generated
has been processed. Therefore calling XSync there reduces performance
with no benefit. (Yes, GetImage is slow anyway, but why make it worse?)

Calling XSync before changing the error handler the first time is
correct, though, as there may be async errors waiting to be read off the
wire, or even reply-free requests waiting in the client's output queue
that could trigger errors once they're flushed.

Or, of course, you could switch to XCB for this one call, and avoid the
extra round-trip due to XSync entirely. ;-) (If you only change this one
call to use XCB then the resulting patch is a fair bit bigger than this
one, and performance wins in Xnest's GetImage are pretty worthless, so
that isn't a very serious suggestion.)

Jamey

On Sat, Oct 05, 2013 at 07:43:21AM +0200, Egbert Eich wrote:
> From: Radek Doulik <rodo at novell.com>
> 
> When an Xnest instance is not viewable it will crash when a client in
> that instance calls GetImage. This is because the Xnest server will
> itself receives a BadMatch error.
> This patch ignores the error. The application which has requested the
> image will receive garbage - this however is fully legal according
> to the specs as obscured areas will always contain garbage if there
> isn't some sort of backing store as discussed in
> https://bugs.freedesktop.org/show_bug.cgi?id=9488
> The applied patch is a version from Dadek Doulik.
> 
> v2: Call XSync() before changing error handlers as suggested by
>     Daniel Stone <daniel at fooishbar.org>.
> 
> Signed-off-by: Egbert Eich <eich at freedesktop.org>
> ---
>  hw/xnest/GCOps.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/hw/xnest/GCOps.c b/hw/xnest/GCOps.c
> index e26a136..7b1956d 100644
> --- a/hw/xnest/GCOps.c
> +++ b/hw/xnest/GCOps.c
> @@ -94,15 +94,29 @@ xnestPutImage(DrawablePtr pDrawable, GCPtr pGC, int depth, int x, int y,
>      }
>  }
>  
> +static int
> +xnestIgnoreErrorHandler (Display     *display,
> +                         XErrorEvent *event)
> +{
> +    return False; /* return value is ignored */
> +}
> +
>  void
>  xnestGetImage(DrawablePtr pDrawable, int x, int y, int w, int h,
>                unsigned int format, unsigned long planeMask, char *pImage)
>  {
>      XImage *ximage;
>      int length;
> +    int (*old_handler)(Display*, XErrorEvent*);
> +
> +    /* we may get BadMatch error when xnest window is minimized */
> +    XSync(xnestDisplay, False);
> +    old_handler = XSetErrorHandler (xnestIgnoreErrorHandler);
>  
>      ximage = XGetImage(xnestDisplay, xnestDrawable(pDrawable),
>                         x, y, w, h, planeMask, format);
> +    XSync(xnestDisplay, False);
> +    XSetErrorHandler(old_handler);
>  
>      if (ximage) {
>          length = ximage->bytes_per_line * ximage->height;
> -- 
> 1.8.1.4
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20131004/3cb8fd33/attachment.pgp>