[PATCH xserver] xfixes: Remove the CursorCurrent array

Adam Jackson ajax at redhat.com
Fri Dec 8 16:57:38 UTC 2017


On Fri, 2017-12-08 at 09:41 +0000, Alan Hourihane wrote:
> On 08/06/17 22:51, Keith Packard wrote:
> > Adam Jackson <ajax at redhat.com> writes:
> > 
> > > We're not wrapping all the ways a cursor can be destroyed, so this array
> > > ends up with stale data. Rather than try harder to wrap more code paths,
> > > just look up the cursor when we need it.
> > 
> > I'm pretty sure it doesn't matter -- DisplayCursor is only ever called
> > while *both* cursors are still valid. Here's the DIX code:
> > 
> >         (*pScreen->DisplayCursor) (pDev, pScreen, cursor);
> >         FreeCursor(pSprite->current, (Cursor) 0);
> >         pSprite->current = RefCursor(cursor);
> > 
> > Note that InitializeSprite also sets pSprite->current *before* calling
> > DisplayCursor, which breaks your assumption. I don't think that matters
> > as it should only be done before a client could possibly know about the
> > device?
> > 
> > I can see why you might want to get rid of the magic array; seems like
> > this should just be using a private in the device.
> 
> So what's happening with this ?
> 
> I've just posted a fix which has been on RedHat's radar for 18 months
> with the same patch

My rhbz folder has 125 new mails in it since I left work yesterday.
Bugs from actual customers (as opposed to random yahoo email addresses)
tend to get prioritized by our processes. I assume you made a typo in
describing the bug as "fixed in RedHat's bugzilla database" and meant
"filed", as the bug has not been closed nor does it contain a patch.

> You can easily crash the Xserver without this fix.

Yes, that's why I posted the patch in the first place. I saw your patch
and did indeed suspect it was the same issue as the one I'd sent;
happened to be working on something else that day, sorry I didn't jump
all over it. Normally the way we say we think a patch is a good idea
is:

Reviewed-by: Adam Jackson <ajax at redhat.com>

Now in practice the set of people who review patches is quite small,
which is a shame, because I'll believe an r-b from just about anybody.
If anyone dislikes the existing pace of development, code review would
be a sincerely welcome contribution.

> You can easily crash the Xserver without this fix.

Thanks for confirming that it works. I've merged my version on style
grounds, getting rid of the array seems like a more robust solution.

- ajax


More information about the xorg-devel mailing list