(submit/recv-fds) os: read file descriptors into client struct at once

Instead of having the request handler ask for fd's one by one, just read them
all into a little array in ClientRec struct. And also automatically clean up
after request had been handled.

Request handlers need to set the entries to -1 if they shouldn't be closed
automatically.

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

Xext/shm.c

@@ -1124,14 +1124,15 @@ ProcShmAttachFd(ClientPtr client)
     REQUEST(xShmAttachFdReq);
     struct stat statb;
 
-    SetReqFds(client, 1);
     REQUEST_SIZE_MATCH(xShmAttachFdReq);
     LEGAL_NEW_RESOURCE(stuff->shmseg, client);
     if ((stuff->readOnly != xTrue) && (stuff->readOnly != xFalse)) {
         client->errorValue = stuff->readOnly;
         return BadValue;
     }
-    fd = ReadFdFromClient(client);
+    fd = client->recv_fd_list[0];
+    client->recv_fd_list[0] = -1;
+
     if (fd < 0)
         return BadMatch;
 
@@ -1435,7 +1436,6 @@ static int _X_COLD
 SProcShmAttachFd(ClientPtr client)
 {
     REQUEST(xShmAttachFdReq);
-    SetReqFds(client, 1);
     REQUEST_SIZE_MATCH(xShmAttachFdReq);
     swapl(&stuff->shmseg);
     return ProcShmAttachFd(client);

dix/dispatch.c

@@ -560,6 +560,14 @@ Dispatch(void)
                             (*client->requestVector[client->majorOp]) (client);
                         currentClient = NULL;
                     }
+
+#if XTRANS_SEND_FDS
+                    /* close down all unused passed fd's */
+                    for (int x=0; x<MAX_CLIENT_RECV_FD; x++) {
+                        close(client->recv_fd_list[x]);
+                        client->recv_fd_list[x] = -1;
+                    }
+#endif
                 }
                 if (!SmartScheduleSignalEnable)
                     SmartScheduleTime = GetTimeInMillis();

dix/dixstruct_priv.h

@@ -16,13 +16,6 @@
 #include "dixstruct.h"
 #include <X11/Xmd.h>
 
-static inline void
-SetReqFds(ClientPtr client, int req_fds) {
-    if (client->req_fds != 0 && req_fds != client->req_fds)
-        LogMessage(X_ERROR, "Mismatching number of request fds %d != %d\n", req_fds, client->req_fds);
-    client->req_fds = req_fds;
-}
-
 /*
  * Scheduling interface
  */

dri3/dri3_request.c

@@ -191,7 +191,6 @@ proc_dri3_pixmap_from_buffer(ClientPtr client)
     CARD32 stride, offset;
     int rc;
 
-    SetReqFds(client, 1);
     REQUEST_SIZE_MATCH(xDRI3PixmapFromBufferReq);
     LEGAL_NEW_RESOURCE(stuff->pixmap, client);
     rc = dixLookupDrawable(&drawable, stuff->drawable, client, M_ANY, DixGetAttrAccess);
@@ -220,7 +219,8 @@ proc_dri3_pixmap_from_buffer(ClientPtr client)
         }
     }
 
-    fd = ReadFdFromClient(client);
+    fd = client->recv_fd_list[0];
+    client->recv_fd_list[0] = -1;
     if (fd < 0)
         return BadValue;
 
@@ -309,7 +309,6 @@ proc_dri3_fence_from_fd(ClientPtr client)
     int fd;
     int status;
 
-    SetReqFds(client, 1);
     REQUEST_SIZE_MATCH(xDRI3FenceFromFDReq);
     LEGAL_NEW_RESOURCE(stuff->fence, client);
 
@@ -317,7 +316,8 @@ proc_dri3_fence_from_fd(ClientPtr client)
     if (status != Success)
         return status;
 
-    fd = ReadFdFromClient(client);
+    fd = client->recv_fd_list[0];
+    client->recv_fd_list[0] = -1;
     if (fd < 0)
         return BadValue;
 
@@ -434,7 +434,6 @@ proc_dri3_pixmap_from_buffers(ClientPtr client)
     int rc;
     int i;
 
-    SetReqFds(client, stuff->num_buffers);
     REQUEST_SIZE_MATCH(xDRI3PixmapFromBuffersReq);
     LEGAL_NEW_RESOURCE(stuff->pixmap, client);
     rc = dixLookupWindow(&window, stuff->window, client, DixGetAttrAccess);
@@ -470,7 +469,7 @@ proc_dri3_pixmap_from_buffers(ClientPtr client)
     }
 
     for (i = 0; i < stuff->num_buffers; i++) {
-        fds[i] = ReadFdFromClient(client);
+        fds[i] = client->recv_fd_list[i];
         if (fds[i] < 0) {
             while (--i >= 0)
                 close(fds[i]);
@@ -609,7 +608,6 @@ proc_dri3_import_syncobj(ClientPtr client)
     int fd;
     int status;
 
-    SetReqFds(client, 1);
     REQUEST_SIZE_MATCH(xDRI3ImportSyncobjReq);
     LEGAL_NEW_RESOURCE(stuff->syncobj, client);
 
@@ -620,7 +618,8 @@ proc_dri3_import_syncobj(ClientPtr client)
 
     screen = drawable->pScreen;
 
-    fd = ReadFdFromClient(client);
+    fd = client->recv_fd_list[0];
+    client->recv_fd_list[0] = -1;
     if (fd < 0)
         return BadValue;
 

include/dixstruct.h

@@ -75,6 +75,9 @@ typedef struct _saveSet {
 
 struct _ClientId;
 
+/* currently largest one in use is 4 */
+#define MAX_CLIENT_RECV_FD	8
+
 typedef struct _Client {
     void *requestBuffer;
     void *osPrivate;             /* for OS layer, including scheduler */
@@ -112,7 +115,9 @@ typedef struct _Client {
 
     DeviceIntPtr clientPtr;
     struct _ClientId *clientIds;
-    int req_fds;
+
+    int recv_fd_count;
+    int recv_fd_list[MAX_CLIENT_RECV_FD];
 } ClientRec;
 
 typedef struct _WorkQueue {

include/os.h

@@ -86,8 +86,6 @@ typedef struct _NewClientRec *NewClientPtr;
 
 extern _X_EXPORT int ReadRequestFromClient(ClientPtr /*client */ );
 
-extern _X_EXPORT int ReadFdFromClient(ClientPtr client);
-
 extern _X_EXPORT int WriteFdToClient(ClientPtr client, int fd, Bool do_close);
 
 extern _X_EXPORT Bool InsertFakeRequest(ClientPtr /*client */ ,

os/io.c

@@ -252,14 +252,6 @@ ReadRequestFromClient(ClientPtr client)
         oc->input = oci;
     }
 
-#if XTRANS_SEND_FDS
-    /* Discard any unused file descriptors */
-    while (client->req_fds > 0) {
-        int req_fd = ReadFdFromClient(client);
-        if (req_fd >= 0)
-            close(req_fd);
-    }
-#endif
     /* advance to start of next request */
 
     oci->bufptr += oci->lenLastReq;
@@ -459,25 +451,23 @@ ReadRequestFromClient(ClientPtr client)
                client->index, req->reqType, req->data, req->length);
     }
 #endif
-    return needed;
-}
-
-int
-ReadFdFromClient(ClientPtr client)
-{
-    int fd = -1;
 
+    /* read the passed fds into Client struct */
+    client->recv_fd_count = 0;
 #if XTRANS_SEND_FDS
-    if (client->req_fds > 0) {
-        OsCommPtr oc = (OsCommPtr) client->osPrivate;
-
-        --client->req_fds;
-        fd = _XSERVTransRecvFd(oc->trans_conn);
-    } else
-        LogMessage(X_ERROR, "Request asks for FD without setting req_fds\n");
+    for (;((client->recv_fd_count < MAX_CLIENT_RECV_FD) &&
+           (client->recv_fd_list[client->recv_fd_count] = _XSERVTransRecvFd(oc->trans_conn)) > 0);
+          client->recv_fd_count++) {}
+    // just in case somebody sent a massive number of fd's
+    int _fd = -1;
+    while ((_fd = _XSERVTransRecvFd(oc->trans_conn)) > 0)
+        close(_fd);
 #endif
 
-    return fd;
+    for (int x=client->recv_fd_count; x<MAX_CLIENT_RECV_FD; x++)
+        client->recv_fd_list[x] = -1;
+
+    return needed;
 }
 
 int