[PATCH xserver] was: Re: Two long-standing server bugs

[Peter Hutterer]

On Tue, Sep 03, 2013 at 09:47:28AM +0200, Thomas Klausner wrote:
> On Sun, Sep 01, 2013 at 01:37:31PM -0400, der Mouse wrote:
> > The first is that CreateCursor (Xlib call XCreatePixmapCursor) with a
> > non-bitmap source pixmap and a None mask is, as I read it, supposed to
> > error out with BadMatch, but doesn't.  This is the one I fixed.
> > 
> > The second is that pixmap cursors created with a None mask get
> > extended, in some cases, to the right with additional pixels in the
> > background colour, usually to a multiple of 32 pixels.  I don't know
> > exactly when this happens; in my experience it correlates (almost?)
> > perfectly with using video hardware that's behind a PCI bus, though I
> > feel reasonably sure that it's more a question of which ddx device code
> > is in use.  This is not, strictly, contrary to the spec, since cursors
> > "may be transformed arbitrarily to meet display limitations", but,
> > since it works correctly (in the cases I've seen) when the client
> > specifies a mask pixmap full of 1s, there clearly is no actual need to
> > mangle cursors specified with a mask of None this way.  So I think
> > calling it a bug is fair.  This is the one I kinda-sorta fixed.
> > 
> > Here are my patches.  Pathnames are based on NetBSD's /usr/xsrc; I
> > trust people here can adapt as necessary. :-)  Anyone is welcome to
> > pick these up, either to be fed back into the main tree or for private
> > use, as desired.  (In case an explicit statement to this effect is
> > helpful, I release any intellectual property rights I may have in these
> > patches into the public domain.)
> > 
> > First, the `missing BadMatch' bug.
> > 
> > --- a/external/mit/xorg-server/dist/dix/dispatch.c
> > +++ b/external/mit/xorg-server/dist/dix/dispatch.c
> > @@ -2863,9 +2863,10 @@ ProcCreateCursor (ClientPtr client)
> >      }
> >      else if (  src->drawable.width != msk->drawable.width
> >  	    || src->drawable.height != msk->drawable.height
> > -	    || src->drawable.depth != 1
> >  	    || msk->drawable.depth != 1)
> >  	return (BadMatch);
> > +    if (src->drawable.depth != 1)
> > +	return (BadMatch);
> >  
> >      width = src->drawable.width;
> >      height = src->drawable.height;
> 
> I've adapted this patch to the current xserver git sources, attached.
> Please review.
>  Thomas

> From 8057fc810fb2815ea4c069ac9c1bc77709afb778 Mon Sep 17 00:00:00 2001
> From: Thomas Klausner <wiz at NetBSD.org>
> Date: Tue, 3 Sep 2013 09:44:18 +0200
> Subject: [PATCH:xserver] Fix bug in cursor handling.
> 
> CreateCursor (Xlib call XCreatePixmapCursor) with a non-bitmap
> source pixmap and a None mask is supposed to error out with BadMatch,
> but didn't.
> 
> From der Mouse <mouse at Rodents-Montreal.ORG>
> 
> Signed-off-by: Thomas Klausner <wiz at NetBSD.org>
> ---
>  dix/dispatch.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/dix/dispatch.c b/dix/dispatch.c
> index 51d0de2..ae3b97b 100644
> --- a/dix/dispatch.c
> +++ b/dix/dispatch.c
> @@ -2874,9 +2874,12 @@ ProcCreateCursor(ClientPtr client)
>      }
>      else if (src->drawable.width != msk->drawable.width
>               || src->drawable.height != msk->drawable.height
> -             || src->drawable.depth != 1 || msk->drawable.depth != 1)
> +             || msk->drawable.depth != 1)
>          return BadMatch;
>  
> +    if (src->drawable.depth != 1)
> +        return (BadMatch);
> +

I'm staring hard at this and struggling to see the change. The previous code
already returned BadMatch for a src depth of != 1. 

Cheers,
   Peter

>      width = src->drawable.width;
>      height = src->drawable.height;
>  
> -- 
> 1.8.4
>