[PATCH] move reference counting out of the UseHWCursor/UseHWCursorARGB functions
Roland Scheidegger
rscheidegger_lists at hispeed.ch
Wed Mar 10 08:08:49 PST 2010
On 10.03.2010 12:02, Michel Dänzer wrote:
> On Tue, 2010-03-09 at 18:38 +0100, Roland Scheidegger wrote:
>> The problem is that the xf86_use_hw_cursor(_argb) functions may get this
>> correctly now, some drivers will replace this with their own functions.
>
> To be pedantic, it's not so much that the drivers 'replace' these as
> that they're generic implementations provided by the server for RandR
> 1.2 capable drivers.
Right. Still, (old) drivers are replacing this generic code with their
own versions.
>
>> It is pretty insane to expect them to do reference counting of the
>> cursor (as an example, look at driver/xf86-video-vmware to see how that
>> looks like as a workaround). There are even places in xserver itself
>> which replace these two functions.
>>
>> FWIW, the segfaults are caused because the reference count of the cursor
>> reached zero, hence the cursor was freed, however
>> xf86CursorEnableDisableFBAccess() brought it back to life from the dead
>> (from the SavedCursor), at this point obviously anything can happen.
>> This patch hence adds reference counting in xf86CursorSetCursor.
>> In theory with this it should be possible to remove the reference
>> counting in the UseHwCursor functions I think, though it should also be
>> safe to keep them.
>> I can't guarantee though this is fully correct as the reference counting
>> looks a bit fishy overall, hopefully it won't create a memleak...
>
> I think it's missing some code in xf86CursorCloseScreen() to prevent a
> possible leak of one CursorRec per server generation.
You're right I suppose (I know I did that in the vmware driver but I
wasn't really sure it was actually necessary).
>
> This can be done simpler as below, right?
Yes, certainly, looks even better.
Roland
>
>
> diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c
> index 896ed37..4b33582 100644
> --- a/hw/xfree86/ramdac/xf86Cursor.c
> +++ b/hw/xfree86/ramdac/xf86Cursor.c
> @@ -129,6 +129,9 @@ xf86CursorCloseScreen(int i, ScreenPtr pScreen)
> if (ScreenPriv->isUp && pScrn->vtSema)
> xf86SetCursor(pScreen, NullCursor, ScreenPriv->x, ScreenPriv->y);
>
> + if (ScreenPriv->CurrentCursor)
> + FreeCursor(ScreenPriv->CurrentCursor, None);
> +
> pScreen->CloseScreen = ScreenPriv->CloseScreen;
> pScreen->QueryBestSize = ScreenPriv->QueryBestSize;
> pScreen->RecolorCursor = ScreenPriv->RecolorCursor;
> @@ -317,6 +320,9 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs,
> if (pDev == inputInfo.pointer ||
> (!pDev->isMaster && pDev->u.master == inputInfo.pointer))
> {
> + pCurs->refcnt++;
> + if (ScreenPriv->CurrentCursor)
> + FreeCursor(ScreenPriv->CurrentCursor, None);
> ScreenPriv->CurrentCursor = pCurs;
> ScreenPriv->x = x;
> ScreenPriv->y = y;
>
>
More information about the xorg-devel
mailing list