On Wed, Sep 04, 2013 at 11:13:14AM -0400, Jasper St. Pierre wrote: > Yeah, this is how I would write it. > > Can you provide a patch, Alan?
I tried to make the code match Alan's suggestion, attached. While there, I fixed a typo in the configure.ac script, also attached. Thomas > > On Wed, Sep 4, 2013 at 2:30 AM, Alan Coopersmith < > [email protected]> wrote: > > > On 09/ 3/13 11:05 PM, Peter Hutterer wrote: > > >> From 8057fc810fb2815ea4c069ac9c1bc77709afb778 Mon Sep 17 00:00:00 2001 > > >> From: Thomas Klausner <[email protected]> > > >> 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 <[email protected]> > > >> > > >> Signed-off-by: Thomas Klausner <[email protected]> > > >> --- > > >> 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. > > > > I couldn't see it from the patch either, but a little more context than > > the patch > > includes helps: > > > > 2867 rc = dixLookupResourceByType((pointer *) &msk, stuff->mask, > > RT_PIXMAP, > > 2868 client, DixReadAccess); > > 2869 if (rc != Success) { > > 2870 if (stuff->mask != None) { > > 2871 client->errorValue = stuff->mask; > > 2872 return rc; > > 2873 } > > 2874 } > > 2875 else if (src->drawable.width != msk->drawable.width > > 2876 || src->drawable.height != msk->drawable.height > > 2877 || src->drawable.depth != 1 || msk->drawable.depth != > > 1) > > 2878 return BadMatch; > > > > So if stuff->mask is None, we do a resource lookup for a pixmap with that > > id, > > which should fail, since it's None (aka 0). > > > > Thus we gp into the check at 2869, but don't match the if at 2870, so don't > > return. Since 2875 is the else clause for the if we already went into, we > > skip over all those checks. > > > > The patch moves the src->drawable.depth check out of the else clause so > > it's > > always checked, not just in the (rc == Success) case for the mask pixmap > > lookup. > > It could even be moved up above the lookup of the mask pixmap, since it > > only > > checks against the cursor image pixmap. > > > > I'm not sure if there's ever a reason to try to lookup a mask id of None, > > otherwise this might be easier to follow as: > > > > /* Find and validate cursor image pixmap */ > > rc = dixLookupResourceByType((pointer *) &src, stuff->source, > > RT_PIXMAP, > > client, DixReadAccess); > > if (rc != Success) { > > client->errorValue = stuff->source; > > return rc; > > } > > > > if (src->drawable.depth != 1) > > return (BadMatch); > > > > /* Find and validate cursor mask pixmap, if one is provided */ > > if (stuff->mask != None) { > > rc = dixLookupResourceByType((pointer *) &msk, stuff->mask, > > RT_PIXMAP, > > client, DixReadAccess); > > if (rc != Success) { > > client->errorValue = stuff->mask; > > return rc; > > } > > > > if (src->drawable.width != msk->drawable.width > > || src->drawable.height != msk->drawable.height > > || msk->drawable.depth != 1) > > return BadMatch; > > } > > else > > msk = NULL; > > > > > > -- > > -Alan Coopersmith- [email protected] > > Oracle Solaris Engineering - http://blogs.oracle.com/alanc > > _______________________________________________ > > [email protected]: X.Org development > > Archives: http://lists.x.org/archives/xorg-devel > > Info: http://lists.x.org/mailman/listinfo/xorg-devel > > > > > > -- > Jasper
>From ac4558c7b5ffec3cd148e992332f3b13a2b9b35a Mon Sep 17 00:00:00 2001 From: Thomas Klausner <[email protected]> Date: Wed, 4 Sep 2013 20:05:51 +0200 Subject: [PATCH:xserver 1/2] 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 <[email protected]>, changed following comments by Alan Coopersmith <[email protected]>. Signed-off-by: Thomas Klausner <[email protected]> --- dix/dispatch.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/dix/dispatch.c b/dix/dispatch.c index 51d0de2..71fda48 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -2864,18 +2864,25 @@ ProcCreateCursor(ClientPtr client) return rc; } - rc = dixLookupResourceByType((pointer *) &msk, stuff->mask, RT_PIXMAP, - client, DixReadAccess); - if (rc != Success) { - if (stuff->mask != None) { + if (src->drawable.depth != 1) + return (BadMatch); + + /* Find and validate cursor mask pixmap, if one is provided */ + if (stuff->mask != None) { + rc = dixLookupResourceByType((pointer *) &msk, stuff->mask, RT_PIXMAP, + client, DixReadAccess); + if (rc != Success) { client->errorValue = stuff->mask; return rc; } + + if (src->drawable.width != msk->drawable.width + || src->drawable.height != msk->drawable.height + || src->drawable.depth != 1 || msk->drawable.depth != 1) + return BadMatch; } - else if (src->drawable.width != msk->drawable.width - || src->drawable.height != msk->drawable.height - || src->drawable.depth != 1 || msk->drawable.depth != 1) - return BadMatch; + else + msk = NULL; width = src->drawable.width; height = src->drawable.height; -- 1.8.4
>From 2bd145cb6aa6ab406bf1c8a0775d9d470675efa1 Mon Sep 17 00:00:00 2001 From: Thomas Klausner <[email protected]> Date: Wed, 4 Sep 2013 20:06:07 +0200 Subject: [PATCH:xserver 2/2] Fix typo in configure warning. Signed-off-by: Thomas Klausner <[email protected]> --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index d27ca23..5159420 100644 --- a/configure.ac +++ b/configure.ac @@ -47,7 +47,7 @@ XORG_WITH_XSLTPROC XORG_ENABLE_UNIT_TESTS XORG_LD_WRAP([optional]) -m4_ifndef([XORG_FONT_MACROS_VERSION], [m4_fatal([must install fontutil 1.1 or later before running autoconf/autogen])]) +m4_ifndef([XORG_FONT_MACROS_VERSION], [m4_fatal([must install font-util 1.1 or later before running autoconf/autogen])]) XORG_FONT_MACROS_VERSION(1.1) dnl this gets generated by autoheader, and thus contains all the defines. we -- 1.8.4
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
