[PATCH xserver] xfixes: Remove the CursorCurrent array

[Alan Hourihane]

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 <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.
>

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.

Alan.