[PATCH xserver] xfixes: Remove the CursorCurrent array

[Alan Hourihane]

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

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.