Two long-standing server bugs

[Mouse]

I recently found two very long-standing server bugs (they date back to
at least X11R4, possibly farther).  I found these in the X shipped with
NetBSD 5.2, which is X11R7 of some stripe - I'm not sure how to get any
more detailed version.  I don't know whether they are present in
bleeding edge x.org servers, but I find basically the same code,
exhibiting the same misbehaviour, in X11R4, so it seems likely to me
that they are.  I've fixed one and semi-fixed the other (I have it
working for the cases I care about, but I feel reasonably sure the
"fix" will break in other cases).

There is a test program for them available from
ftp.rodents-montreal.org, in /mouse/X/cursorbug.c; it has an extensive
comment header explaining what the bugs in question are, and the code
is deliberately minimalist, intended to be easy to understand.

I wrote to NetBSD's tech-x11 list about this; someone there recommended
I repeat the report here.  (There's an archived copy of my tech-x11
mail at http://mail-index.netbsd.org/tech-x11/2013/08/31/msg001293.html
in case anyone wants to see it.)

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;

Next, the cursor-extension bug.  In my use cases, such cursor creation
is a rare enough event that I didn't bother complicating the code for
the sake of speed, and I feel moderately sure this fix will not work
right in servers using other bitmap bit orders and/or byte orders.
Arguably this is not the right place to fix this (it seems to me the
ddx layers should be paying attention to the width and height values,
and I suspect that's what's happening in the cases where I didn't see
this bug manifest).  But the comment about extending with 0s to help
ddx cursor handling (not quoted here; it's just after the last line of
context) makes me think the internal interface contract may be such
that dix is supposed to provide a mask padded with 0 bits to ddx.  (And
my code has some historical issues which I really ought to fix, notably
the way the x loop is structured.)

--- a/external/mit/xorg-server/dist/dix/dispatch.c
+++ b/external/mit/xorg-server/dist/dix/dispatch.c
@@ -2890,9 +2890,17 @@ ProcCreateCursor (ClientPtr client)
 					 XYPixmap, 1, (pointer)srcbits);
     if ( msk == (PixmapPtr)NULL)
     {
-	unsigned char *bits = mskbits;
-	while (--n >= 0)
-	    *bits++ = ~0;
+      register int y;
+      register int x;
+      register int w;
+      w = BitmapBytePad(width);
+      for (y=height-1;y>=0;y--)
+       { for (x=(w*8)-1;x>=0;x--)
+	  { if (x < width)
+	     { mskbits[(y*w)+(x>>3)] |= 1 << (x & 7);
+	     }
+	  }
+       }
     }
     else
     {

/~\ The ASCII				  Mouse
\ / Ribbon Campaign
 X  Against HTML		mouse at rodents-montreal.org
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B