[PATCH] damage: Remove the 'damage window' resource type

[Adam Jackson]

On Mon, 2011-03-28 at 11:03 -0700, Keith Packard wrote:
> On Mon, 28 Mar 2011 10:56:08 -0400, Adam Jackson <ajax at redhat.com> wrote:
> 
> > No code to create objects of this type ever existed.
> 
> I suspect that's a bug then -- the damage object is going to hang around
> with a dead pointer to the window when it is destroyed.
> 
> Here's a completely untested patch that hooks the window id to the
> damage record so that it is destroyed when the window is destroyed:
> 
> I think this gets the ENOMEM case right -- AddResource will call 
> FreeDamageExtWin if it fails, which will call FreeResource on the damage
> ID, freeing the damage structure.

This may be more correct in that it cleans up the Damage.  I think it's
less correct in that you break existing clients.  FreeDamageExtWin()
deletes the Damage XID, but the spec mentions nothing about magically
garbage-collecting Damages just because their drawable went away.  From
a quick check, it looks like mutter pushes an error handler around
XDamageDestroy(), but compiz and kwin do not.

Given that, I think we have to make it so damage actions against a
Damage that has lost its Drawable simply fizzle, and continue to leave
cleanup to the clients.  Compiled, not tested:

diff --git a/damageext/damageext.c b/damageext/damageext.c
index 754383d..72611ec 100644
--- a/damageext/damageext.c
+++ b/damageext/damageext.c
@@ -30,7 +30,7 @@
 static unsigned char	DamageReqCode;
 static int		DamageEventBase;
 static RESTYPE		DamageExtType;
-static RESTYPE		DamageExtWinType;
+static RESTYPE		DamageExtDrawableType;
 
 static DevPrivateKeyRec DamageClientPrivateKeyRec;
 #define DamageClientPrivateKey (&DamageClientPrivateKeyRec)
@@ -89,6 +89,9 @@ DamageExtReport (DamagePtr pDamage, RegionPtr pRegion, void *closure)
 {
     DamageExtPtr    pDamageExt = closure;
 
+    if (pDamageExt->drawable == None)
+	return;
+
     switch (pDamageExt->level) {
     case DamageReportRawRegion:
     case DamageReportDeltaRegion:
@@ -220,6 +223,9 @@ ProcDamageCreate (ClientPtr client)
     DamageSetReportAfterOp (pDamageExt->pDamage, TRUE);
     DamageRegister (pDamageExt->pDrawable, pDamageExt->pDamage);
 
+    if (!AddResource(pDrawable->id, DamageExtDrawableType, pDamageExt))
+	return BadAlloc;
+
     if (pDrawable->type == DRAWABLE_WINDOW)
     {
 	pRegion = &((WindowPtr) pDrawable)->borderClip;
@@ -446,7 +452,7 @@ FreeDamageExt (pointer value, XID did)
      */
     pDamageExt->id = 0;
     if (WindowDrawable(pDamageExt->pDrawable->type))
-	FreeResourceByType (pDamageExt->pDrawable->id, DamageExtWinType, TRUE);
+	FreeResourceByType (pDamageExt->pDrawable->id, DamageExtDrawableType, TRUE);
     if (pDamageExt->pDamage)
     {
 	DamageUnregister (pDamageExt->pDrawable, pDamageExt->pDamage);
@@ -457,12 +463,12 @@ FreeDamageExt (pointer value, XID did)
 }
 
 static int
-FreeDamageExtWin (pointer value, XID wid)
+FreeDamageExtDrawable (pointer value, XID wid)
 {
     DamageExtPtr    pDamageExt = (DamageExtPtr) value;
 
-    if (pDamageExt->id)
-	FreeResource (pDamageExt->id, RT_NONE);
+    pDamageExt->drawable = None;
+
     return Success;
 }
 
@@ -497,8 +503,9 @@ DamageExtensionInit(void)
     if (!DamageExtType)
 	return;
 
-    DamageExtWinType = CreateNewResourceType (FreeDamageExtWin, "DamageExtWin");
-    if (!DamageExtWinType)
+    DamageExtDrawableType = CreateNewResourceType (FreeDamageExtDrawable,
+						   "DamageExtDrawable");
+    if (!DamageExtDrawableType)
 	return;
 
     if (!dixRegisterPrivateKey(&DamageClientPrivateKeyRec, PRIVATE_CLIENT, sizeof (DamageClientRec)))

---

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110329/52df4688/attachment.pgp>