On 08/12/17 17:04, Alan Hourihane wrote: > On 08/12/17 16:57, Adam Jackson wrote: >> On Fri, 2017-12-08 at 09:41 +0000, Alan Hourihane wrote: >>> On 08/06/17 22:51, Keith Packard wrote: >>>> Adam Jackson <[email protected]> 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 <[email protected]> >> >> 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. >> > I didn't say I tested your version. The version posted on the RedHat > Bugzilla database works and seems far more robust to me given Keith's > comments. >
But I've gone ahead and tested your fix, and it works too. So I'm fine with this. Can we get this merged to the stable branches ? Alan. _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
