[PATCH 1/4] dix: Update element count in FreeResource*()

[Kristian Høgsberg]

FreeResource() keeps clientTable[cid].elements up to date with the
number of resources allocated to the client.  The other free
resource functions (FreeResourceByType(),
FreeClientNeverRetainResources() and FreeClientResources()) don't
maintain this invariant.

Typically, the only consequence is that the element count is too high
and we end up allocating the hash table bigger than necessary.  However,
FreeResource() also relies on the element count to restart the search if
the list of resources has been changed during a resource destruction
callback.  Since FreeResourceByType() doesn't update the count, if we call
that from a resource destruction callback from FreeResource(), the
loop isn't restarted and we end up following an invalid next pointer.

Furthermore, LookupClientResourceComplex() and
FreeClientNeverRetainResources() don't use the element count to detect
if a callback deleted a resource and may end up following an invalid
next pointer if the resource system is called into recursively.

Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
---
 dix/resource.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/dix/resource.c b/dix/resource.c
index 91d0cfb..048fad1 100644
--- a/dix/resource.c
+++ b/dix/resource.c
@@ -589,6 +589,7 @@ FreeResourceByType(XID id, RESTYPE type, Bool skipFree)
 			      res->value, TypeNameString(res->type));
 #endif		    		    
 		*prev = res->next;
+		clientTable[cid].elements--;
 
 		CallResourceStateCallback(ResourceStateFreeing, res);
 
@@ -707,20 +708,25 @@ LookupClientResourceComplex(
     ResourcePtr *resources;
     ResourcePtr this, next;
     pointer value;
-    int i;
+    int i, elements;
+    int *eltptr;
 
     if (!client)
 	client = serverClient;
 
     resources = clientTable[client->index].resources;
+    eltptr = &clientTable[client->index].elements;
     for (i = 0; i < clientTable[client->index].buckets; i++) {
         for (this = resources[i]; this; this = next) {
 	    next = this->next;
 	    if (!type || this->type == type) {
 		/* workaround func freeing the type as DRI1 does */
 		value = this->value;
+		elements = *eltptr;
 		if((*func)(value, this->id, cdata))
 		    return value;
+		if (*eltptr != elements)
+		    next = resources[i]; /* start over */
 	    }
 	}
     }
@@ -734,12 +740,14 @@ FreeClientNeverRetainResources(ClientPtr client)
     ResourcePtr *resources;
     ResourcePtr this;
     ResourcePtr *prev;
-    int j;
+    int j, elements;
+    int *eltptr;
 
     if (!client)
 	return;
 
     resources = clientTable[client->index].resources;
+    eltptr = &clientTable[client->index].elements;
     for (j=0; j < clientTable[client->index].buckets; j++) 
     {
 	prev = &resources[j];
@@ -753,11 +761,15 @@ FreeClientNeverRetainResources(ClientPtr client)
 			      this->value, TypeNameString(this->type));
 #endif		    
 		*prev = this->next;
+		clientTable[client->index].elements--;
 
 		CallResourceStateCallback(ResourceStateFreeing, this);
 
+		elements = *eltptr;
 		(*DeleteFuncs[rtype & TypeMask])(this->value, this->id);
 		xfree(this);
+		if (*eltptr != elements)
+		    prev = &resources[j]; /* prev may no longer be valid */
 	    }
 	    else
 		prev = &this->next;
@@ -804,6 +816,7 @@ FreeClientResources(ClientPtr client)
 			  this->value, TypeNameString(this->type));
 #endif		    
 	    *head = this->next;
+	    clientTable[client->index].elements--;
 
 	    CallResourceStateCallback(ResourceStateFreeing, this);
 
-- 
1.7.0.1