dix: add dixClientForXID()

Retrieves the ClientPtr for the owner of given resource.
This way reducing the sites directly accessing clients[] array.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

Xext/panoramiX.c

@@ -353,7 +353,7 @@ PanoramiXFindIDByScrnum(RESTYPE type, XID id, int screen)
     data.screen = screen;
     data.id = id;
 
-    return LookupClientResourceComplex(clients[dixClientIdForXID(id)], type,
+    return LookupClientResourceComplex(dixClientForXID(id), type,
                                        XineramaFindIDByScrnum, &data);
 }
 

Xext/security.c

@@ -743,7 +743,6 @@ SecurityResource(CallbackListPtr *pcbl, void *unused, void *calldata)
 {
     XaceResourceAccessRec *rec = calldata;
     SecurityStateRec *subj, *obj;
-    int cid = dixClientIdForXID(rec->id);
     Mask requested = rec->access_mode;
     Mask allowed = SecurityResourceMask;
 
@@ -758,8 +757,12 @@ SecurityResource(CallbackListPtr *pcbl, void *unused, void *calldata)
     if (rec->rtype == X11_RESTYPE_WINDOW)
         allowed |= SecurityWindowExtraMask;
 
+    ClientPtr owner = dixClientForXID(rec->id);
+    if (!owner)
+        goto denied;
+
     /* special checks for server-owned resources */
-    if (cid == 0) {
+    if (owner == serverClient) {
         if (rec->rtype & RC_DRAWABLE)
             /* additional operations allowed on root windows */
             allowed |= SecurityRootWindowExtraMask;
@@ -773,15 +776,15 @@ SecurityResource(CallbackListPtr *pcbl, void *unused, void *calldata)
             allowed |= DixReadAccess;
     }
 
-    if (clients[cid] != NULL) {
+    obj = dixLookupPrivate(&owner->devPrivates, stateKey);
-        obj = dixLookupPrivate(&clients[cid]->devPrivates, stateKey);
     if (SecurityDoCheck(subj, obj, requested, allowed) == Success)
         return;
-    }
 
+denied:
     SecurityAudit("Security: denied client %d access %lx to resource 0x%lx "
                   "of client %d on request %s\n", rec->client->index,
-                  (unsigned long)requested, (unsigned long)rec->id, cid,
+                  (unsigned long)requested, (unsigned long)rec->id,
+                  dixClientIdForXID(rec->id),
                   SecurityLookupRequestName(rec->client));
     rec->status = BadAccess;    /* deny access */
 }

Xext/xres.c

@@ -307,10 +307,10 @@ ProcXResQueryClientResources(ClientPtr client)
 
     REQUEST_SIZE_MATCH(xXResQueryClientResourcesReq);
 
-    int clientID = dixClientIdForXID(stuff->xid);
+    ClientPtr resClient = dixClientForXID(stuff->xid);
 
-    if ((clientID >= currentMaxClients) || !clients[clientID] ||
+    if ((!resClient) ||
-        (XaceHookClientAccess(client, clients[clientID], DixReadAccess)
+        (XaceHookClientAccess(client, resClient, DixReadAccess)
                               != Success)) {
         client->errorValue = stuff->xid;
         return BadValue;
@@ -318,7 +318,7 @@ ProcXResQueryClientResources(ClientPtr client)
 
     counts = calloc(lastResourceType + 1, sizeof(int));
 
-    FindAllClientResources(clients[clientID], ResFindAllRes, counts);
+    FindAllClientResources(resClient, ResFindAllRes, counts);
 
     num_types = 0;
 
@@ -380,22 +380,19 @@ ProcXResQueryClientPixmapBytes(ClientPtr client)
 {
     REQUEST(xXResQueryClientPixmapBytesReq);
     xXResQueryClientPixmapBytesReply rep;
-    unsigned long bytes;
 
     REQUEST_SIZE_MATCH(xXResQueryClientPixmapBytesReq);
 
-    int clientID = dixClientIdForXID(stuff->xid);
+    ClientPtr owner = dixClientForXID(stuff->xid);
-
+    if ((!owner) ||
-    if ((clientID >= currentMaxClients) || !clients[clientID] ||
+        (XaceHookClientAccess(client, owner, DixReadAccess)
-        (XaceHookClientAccess(client, clients[clientID], DixReadAccess)
                               != Success)) {
         client->errorValue = stuff->xid;
         return BadValue;
     }
 
-    bytes = 0;
+    unsigned long bytes = 0;
-
+    FindAllClientResources(owner, ResFindResourcePixmaps,
-    FindAllClientResources(clients[clientID], ResFindResourcePixmaps,
                            (void *) (&bytes));
 
     rep = (xXResQueryClientPixmapBytesReply) {
@@ -555,12 +552,11 @@ ConstructClientIds(ClientPtr client,
                 }
             }
         } else {
-            int clientID = dixClientIdForXID(specs[specIdx].client);
+            ClientPtr owner = dixClientForXID(specs[specIdx].client);
-
+            if (owner &&
-            if ((clientID < currentMaxClients) && clients[clientID] &&
+                (XaceHookClientAccess(client, owner, DixReadAccess)
-                (XaceHookClientAccess(client, clients[clientID], DixReadAccess)
                                       != Success)) {
-                if (!ConstructClientIdValue(client, clients[clientID],
+                if (!ConstructClientIdValue(client, owner,
                                             specs[specIdx].mask, ctx)) {
                     return BadAlloc;
                 }
@@ -897,11 +893,8 @@ ConstructResourceBytesByResource(XID aboutClient, ConstructResourceBytesCtx *ctx
     for (specIdx = 0; specIdx < ctx->numSpecs; ++specIdx) {
         xXResResourceIdSpec *spec = ctx->specs + specIdx;
         if (spec->resource) {
-            int cid = dixClientIdForXID(spec->resource);
+            ClientPtr client = dixClientForXID(spec->resource);
-            if (cid < currentMaxClients &&
+            if (client && (aboutClient == None || aboutClient == client->index)) {
-                (aboutClient == None || cid == aboutClient)) {
-                ClientPtr client = clients[cid];
-                if (client) {
                 ctx->curSpec = spec;
                 FindAllClientResources(client,
                                        AddResourceSizeValueByResource,
@@ -910,7 +903,6 @@ ConstructResourceBytesByResource(XID aboutClient, ConstructResourceBytesCtx *ctx
         }
     }
 }
-}
 
 /** @brief Build the resource size response for the given client
            (or all if not specified) per the parameters set up
@@ -925,16 +917,12 @@ ConstructResourceBytes(XID aboutClient,
                        ConstructResourceBytesCtx *ctx)
 {
     if (aboutClient) {
-        int clientIdx = dixClientIdForXID(aboutClient);
+        ClientPtr client = dixClientForXID(aboutClient);
-        ClientPtr client = NullClient;
+        if (!client) {
-
-        if ((clientIdx >= currentMaxClients) || !clients[clientIdx]) {
             ctx->sendClient->errorValue = aboutClient;
             return BadValue;
         }
 
-        client = clients[clientIdx];
-
         ConstructClientResourceBytes(client, ctx);
         ConstructResourceBytesByResource(aboutClient, ctx);
     } else {

Xext/xselinux_hooks.c

@@ -632,7 +632,10 @@ SELinuxResource(CallbackListPtr *pcbl, void *unused, void *calldata)
     if (offset < 0) {
         /* No: use the SID of the owning client */
         class = SECCLASS_X_RESOURCE;
-        privatePtr = &clients[dixClientIdForXID(rec->id)]->devPrivates;
+        ClientPtr owner = dixClientForXID(rec->id);
+        if (!owner)
+            return;
+        privatePtr = &owner->devPrivates;
         obj = dixLookupPrivate(privatePtr, objectKey);
     }
     else {

composite/compalloc.c

@@ -431,7 +431,7 @@ compFreeClientSubwindows(WindowPtr pWin, XID id)
         return;
     for (prev = &csw->clients; (ccw = *prev); prev = &ccw->next) {
         if (ccw->id == id) {
-            ClientPtr pClient = clients[dixClientIdForXID(id)];
+            ClientPtr pClient = dixClientForXID(id);
 
             *prev = ccw->next;
             if (ccw->update == CompositeRedirectManual) {
@@ -500,9 +500,8 @@ compRedirectOneSubwindow(WindowPtr pParent, WindowPtr pWin)
     if (!csw)
         return Success;
     for (ccw = csw->clients; ccw; ccw = ccw->next) {
-        int ret = compRedirectWindow(clients[dixClientIdForXID(ccw->id)],
+        int ret = compRedirectWindow(dixClientForXID(ccw->id),
                                      pWin, ccw->update);
-
         if (ret != Success)
             return ret;
     }
@@ -522,9 +521,8 @@ compUnredirectOneSubwindow(WindowPtr pParent, WindowPtr pWin)
     if (!csw)
         return Success;
     for (ccw = csw->clients; ccw; ccw = ccw->next) {
-        int ret = compUnredirectWindow(clients[dixClientIdForXID(ccw->id)],
+        int ret = compUnredirectWindow(dixClientForXID(ccw->id),
                                        pWin, ccw->update);
-
         if (ret != Success)
             return ret;
     }

composite/compwindow.c

@@ -575,7 +575,7 @@ compCreateWindow(WindowPtr pWin)
             (*pScreen->SetWindowPixmap) (pWin, parent_pixmap);
         if (csw)
             for (ccw = csw->clients; ccw; ccw = ccw->next)
-                compRedirectWindow(clients[dixClientIdForXID(ccw->id)],
+                compRedirectWindow(dixClientForXID(ccw->id),
                                    pWin, ccw->update);
         if (compImplicitRedirect(pWin, pWin->parent))
             compRedirectWindow(serverClient, pWin, CompositeRedirectAutomatic);

dix/grabs.c

@@ -77,7 +77,6 @@ SOFTWARE.
 void
 PrintDeviceGrabInfo(DeviceIntPtr dev)
 {
-    ClientPtr client;
     LocalClientCredRec *lcc;
     int i, j;
     GrabInfoPtr devGrab = &dev->deviceGrab;
@@ -89,7 +88,7 @@ PrintDeviceGrabInfo(DeviceIntPtr dev)
            (grab->grabtype == XI2) ? "xi2" :
            ((grab->grabtype == CORE) ? "core" : "xi1"), dev->name, dev->id);
 
-    client = clients[dixClientIdForXID(grab->resource)];
+    ClientPtr client = dixClientForXID(grab->resource);
     if (client) {
         pid_t clientpid = GetClientPid(client);
         const char *cmdname = GetClientCmdName(client);
@@ -173,7 +172,6 @@ void
 UngrabAllDevices(Bool kill_client)
 {
     DeviceIntPtr dev;
-    ClientPtr client;
 
     ErrorF("Ungrabbing all devices%s; grabs listed below:\n",
            kill_client ? " and killing their owners" : "");
@@ -182,7 +180,7 @@ UngrabAllDevices(Bool kill_client)
         if (!dev->deviceGrab.grab)
             continue;
         PrintDeviceGrabInfo(dev);
-        client = clients[dixClientIdForXID(dev->deviceGrab.grab->resource)];
+        ClientPtr client = dixClientForXID(dev->deviceGrab.grab->resource);
         if (!kill_client || !client || client->clientGone)
             dev->deviceGrab.DeactivateGrab(dev);
         if (kill_client)

dix/lookup.c

@@ -16,26 +16,26 @@ ClientPtr dixClientForWindow(WindowPtr pWin) {
     if (!pWin)
         return NullClient;
 
-    return clients[dixClientIdForXID(pWin->drawable.id)];
+    return dixClientForXID(pWin->drawable.id);
 }
 
 ClientPtr dixClientForGrab(GrabPtr pGrab) {
     if (!pGrab)
         return NullClient;
 
-    return clients[dixClientIdForXID(pGrab->resource)];
+    return dixClientForXID(pGrab->resource);
 }
 
 ClientPtr dixClientForInputClients(InputClientsPtr pInputClients) {
     if (!pInputClients)
         return NullClient;
 
-    return clients[dixClientIdForXID(pInputClients->resource)];
+    return dixClientForXID(pInputClients->resource);
 }
 
 ClientPtr dixClientForOtherClients(OtherClientsPtr pOtherClients) {
     if (!pOtherClients)
         return NullClient;
 
-    return clients[dixClientIdForXID(pOtherClients->resource)];
+    return dixClientForXID(pOtherClients->resource);
 }

dix/resource_priv.h

@@ -66,4 +66,20 @@ static inline int dixClientIdForXID(XID xid) {
     return ((int)(CLIENT_BITS(xid) >> CLIENTOFFSET));
 }
 
+/*
+ * @brief retrieve client pointer from XID
+ *
+ * XIDs carry the ID of the client who created/owns the resource in upper bits.
+ * (every client so is assigned a range of XIDs it may use for resource creation)
+ *
+ * @param XID the ID of the resource whose client is retrieved
+ * @return pointer to ClientRec structure or NullClient (NULL)
+ */
+static inline ClientPtr dixClientForXID(XID xid) {
+    const int idx = dixClientIdForXID(xid);
+    if (idx < MAXCLIENTS)
+        return clients[idx];
+    return NullClient;
+}
+
 #endif /* _XSERVER_DIX_RESOURCE_PRIV_H */

glx/glxcmds.c

@@ -2505,7 +2505,9 @@ void
 __glXsendSwapEvent(__GLXdrawable *drawable, int type, CARD64 ust,
                    CARD64 msc, CARD32 sbc)
 {
-    ClientPtr client = clients[dixClientIdForXID(drawable->drawId)];
+    ClientPtr client = dixClientForXID(drawable->drawId);
+    if (!client)
+        return;
 
     xGLXBufferSwapComplete2 wire =  {
         .type = __glXEventBase + GLX_BufferSwapComplete

hw/xwayland/xwayland-pixmap.h

@@ -50,7 +50,7 @@ Bool xwl_pixmap_init(void);
 static inline Bool
 xwl_is_client_pixmap(PixmapPtr pixmap)
 {
-    return clients[dixClientIdForXID(pixmap->drawable.id)] != serverClient;
+    return dixClientForXID(pixmap->drawable.id) != serverClient;
 }
 
 #endif /* XWAYLAND_PIXMAP_H */

record/record.c

@@ -866,7 +866,7 @@ RecordInstallHooks(RecordClientsAndProtocolPtr pRCAP, XID oneclient)
             if (pRCAP->pRequestMajorOpSet) {
                 RecordSetIteratePtr pIter = NULL;
                 RecordSetInterval interval;
-                ClientPtr pClient = clients[dixClientIdForXID(client)];
+                ClientPtr pClient = dixClientForXID(client);
 
                 if (pClient && !RecordClientPrivate(pClient)) {
                     RecordClientPrivatePtr pClientPriv;
@@ -949,7 +949,7 @@ RecordUninstallHooks(RecordClientsAndProtocolPtr pRCAP, XID oneclient)
     while (client) {
         if (client != XRecordFutureClients) {
             if (pRCAP->pRequestMajorOpSet) {
-                ClientPtr pClient = clients[dixClientIdForXID(client)];
+                ClientPtr pClient = dixClientForXID(client);
                 int c;
                 Bool otherRCAPwantsProcVector = FALSE;
                 RecordClientPrivatePtr pClientPriv = NULL;
@@ -1143,7 +1143,6 @@ RecordSanityCheckClientSpecifiers(ClientPtr client, XID *clientspecs,
                                   int nspecs, XID errorspec)
 {
     int i;
-    int clientIndex;
     int rc;
     void *value;
 
@@ -1154,10 +1153,10 @@ RecordSanityCheckClientSpecifiers(ClientPtr client, XID *clientspecs,
             continue;
         if (errorspec && (CLIENT_BITS(clientspecs[i]) == errorspec))
             return BadMatch;
-        clientIndex = dixClientIdForXID(clientspecs[i]);
+        ClientPtr pClient = dixClientForXID(clientspecs[i]);
-        if (clientIndex && clients[clientIndex] &&
+        if (pClient && pClient->index != 0 &&
-            clients[clientIndex]->clientState == ClientStateRunning) {
+            pClient->clientState == ClientStateRunning) {
-            if (clientspecs[i] == clients[clientIndex]->clientAsMask)
+            if (clientspecs[i] == pClient->clientAsMask)
                 continue;
             rc = dixLookupResourceByClass(&value, clientspecs[i], RC_ANY,
                                           client, DixGetAttrAccess);