Re: [PATCH v2] [xorg/xserver] mi/misprite: use memory management provided by dixRegisterPrivateKey
On Mon, 14 Mar 2011 14:03:18 -0400, Adam Jackson wrote: > Merged to xserver-next. I had this merged just before leaving for lunch... -- keith.pack...@intel.com pgpncthHX13yB.pgp Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2] [xorg/xserver] mi/misprite: use memory management provided by dixRegisterPrivateKey
On Wed, 9 Mar 2011 17:29:14 +0200, Erkki Seppälä wrote: Merged. a19771e..a8146f6 master -> master -- keith.pack...@intel.com pgp7T9458kC1t.pgp Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2] [xorg/xserver] mi/misprite: use memory management provided by dixRegisterPrivateKey
On 3/9/11 11:55 AM, Daniel Stone wrote: So this looks fine to me. Thanks! There are many other potential fixes just like this one lurking in misprite.c and mipointer.c if that horrible mess hasn't scared you off yet. Reviewed-by: Daniel Stone Merged to xserver-next. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2] [xorg/xserver] mi/misprite: use memory management provided by dixRegisterPrivateKey
Hi, On Wed, Mar 09, 2011 at 05:29:14PM +0200, Erkki Seppälä wrote: > The record allocated by miSpriteDeviceCursorInitialize was not being > released. > > This patch makes misprite use dixRegisterPrivateKey with the record > size argument, which handles the memory management > issues. miSpriteDeviceCursorInitialize is restructured to initialize > pCursorInfo only if miDCDeviceInitialize succeeds. The record itself > is zeroed on cleanup to ensure that the assumptions in the code still > hold. > > Reviewed-by: Rami Ylimäki > Signed-off-by: Erkki Seppälä Looks fine to me. miSpriteDeviceCursorInitialize gets called as spriteFuncs->DeviceCursorInitialize from miPointerDeviceInitialize, which gets called as pScreen->DeviceCursorInitialize from ActivateDevice. Similarly, miSpriteDeviceCursorCleanup only gets called after a DEVICE_CLOSE from CloseDevice(), and we shouldn't get any more events after that; AccelCleanupProc doesn't post any events either, just, er, cleans up. So this looks fine to me. Thanks! There are many other potential fixes just like this one lurking in misprite.c and mipointer.c if that horrible mess hasn't scared you off yet. Reviewed-by: Daniel Stone Cheers, Daniel signature.asc Description: Digital signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2] [xorg/xserver] mi/misprite: use memory management provided by dixRegisterPrivateKey
Oops, I intended to send this to you instead of just Keith and the list - too fast with command line history - I hope you notice it at least this way. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH v2] [xorg/xserver] mi/misprite: use memory management provided by dixRegisterPrivateKey
The record allocated by miSpriteDeviceCursorInitialize was not being
released.
This patch makes misprite use dixRegisterPrivateKey with the record
size argument, which handles the memory management
issues. miSpriteDeviceCursorInitialize is restructured to initialize
pCursorInfo only if miDCDeviceInitialize succeeds. The record itself
is zeroed on cleanup to ensure that the assumptions in the code still
hold.
Reviewed-by: Rami Ylimäki
Signed-off-by: Erkki Seppälä
---
mi/misprite.c | 41 +++--
1 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/mi/misprite.c b/mi/misprite.c
index 770951e..c9d0180 100644
--- a/mi/misprite.c
+++ b/mi/misprite.c
@@ -308,7 +308,7 @@ miSpriteInitialize (ScreenPtr pScreen,
if (!dixRegisterPrivateKey(&miSpriteScreenKeyRec, PRIVATE_SCREEN, 0))
return FALSE;
-if (!dixRegisterPrivateKey(&miSpriteDevPrivatesKeyRec, PRIVATE_DEVICE, 0))
+if (!dixRegisterPrivateKey(&miSpriteDevPrivatesKeyRec, PRIVATE_DEVICE,
sizeof(miCursorInfoRec)))
return FALSE;
pScreenPriv = malloc(sizeof (miSpriteScreenRec));
@@ -860,38 +860,35 @@ miSpriteMoveCursor (DeviceIntPtr pDev, ScreenPtr pScreen,
int x, int y)
static Bool
miSpriteDeviceCursorInitialize(DeviceIntPtr pDev, ScreenPtr pScreen)
{
-miCursorInfoPtr pCursorInfo;
-int ret = FALSE;
-
-pCursorInfo = malloc(sizeof(miCursorInfoRec));
-if (!pCursorInfo)
-return FALSE;
-
-pCursorInfo->pCursor = NULL;
-pCursorInfo->x = 0;
-pCursorInfo->y = 0;
-pCursorInfo->isUp = FALSE;
-pCursorInfo->shouldBeUp = FALSE;
-pCursorInfo->pCacheWin = NullWindow;
-pCursorInfo->isInCacheWin = FALSE;
-pCursorInfo->checkPixels = TRUE;
-pCursorInfo->pScreen = FALSE;
+int ret = miDCDeviceInitialize(pDev, pScreen);
-ret = miDCDeviceInitialize(pDev, pScreen);
-if (!ret)
+if (ret)
{
-free(pCursorInfo);
-pCursorInfo = NULL;
+miCursorInfoPtr pCursorInfo;
+pCursorInfo = dixLookupPrivate(&pDev->devPrivates,
miSpriteDevPrivatesKey);
+pCursorInfo->pCursor = NULL;
+pCursorInfo->x = 0;
+pCursorInfo->y = 0;
+pCursorInfo->isUp = FALSE;
+pCursorInfo->shouldBeUp = FALSE;
+pCursorInfo->pCacheWin = NullWindow;
+pCursorInfo->isInCacheWin = FALSE;
+pCursorInfo->checkPixels = TRUE;
+pCursorInfo->pScreen = FALSE;
}
-dixSetPrivate(&pDev->devPrivates, miSpriteDevPrivatesKey, pCursorInfo);
+
return ret;
}
static void
miSpriteDeviceCursorCleanup(DeviceIntPtr pDev, ScreenPtr pScreen)
{
+miCursorInfoPtr pCursorInfo = dixLookupPrivate(&pDev->devPrivates,
miSpriteDevPrivatesKey);
+
if (DevHasCursor(pDev))
miDCDeviceCleanup(pDev, pScreen);
+
+memset(pCursorInfo, 0, sizeof(miCursorInfoRec));
}
/*
--
1.7.0.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
