[PATCH xserver 7/7] glx: Large commands are context state, not client state

Adam Jackson ajax at redhat.com
Wed Jan 10 18:05:48 UTC 2018


There's no reason a multithreaded client shouldn't be allowed to
interleave other requests (for other contexts) with a RenderLarge. Move
the check into __glXForceCurrent, and store the state in the context not
the client.

Signed-off-by: Adam Jackson <ajax at redhat.com>
---
 glx/glxcmds.c    | 70 ++++++++++++++++++++++++++++++++------------------------
 glx/glxcontext.h | 10 ++++++++
 glx/glxext.c     | 31 +++++++++----------------
 glx/glxext.h     |  1 -
 glx/glxserver.h  | 10 --------
 5 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 66018f6de..c1b09d5a1 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -1984,6 +1984,18 @@ __glXDisp_GetDrawableAttributesSGIX(__GLXclientState * cl, GLbyte * pc)
 ** client library to send batches of GL rendering commands.
 */
 
+/*
+** Reset state used to keep track of large (multi-request) commands.
+*/
+static void
+ResetLargeCommandStatus(__GLXcontext *cx)
+{
+    cx->largeCmdBytesSoFar = 0;
+    cx->largeCmdBytesTotal = 0;
+    cx->largeCmdRequestsSoFar = 0;
+    cx->largeCmdRequestsTotal = 0;
+}
+
 /*
 ** Execute all the drawing commands in a request.
 */
@@ -2115,8 +2127,6 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * pc)
 
     glxc = __glXForceCurrent(cl, req->contextTag, &error);
     if (!glxc) {
-        /* Reset in case this isn't 1st request. */
-        __glXResetLargeCommandStatus(cl);
         return error;
     }
     if (safe_pad(req->dataBytes) < 0)
@@ -2129,12 +2139,12 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * pc)
     if ((req->length << 2) != safe_pad(dataBytes) + sz_xGLXRenderLargeReq) {
         client->errorValue = req->length;
         /* Reset in case this isn't 1st request. */
-        __glXResetLargeCommandStatus(cl);
+        ResetLargeCommandStatus(glxc);
         return BadLength;
     }
     pc += sz_xGLXRenderLargeReq;
 
-    if (cl->largeCmdRequestsSoFar == 0) {
+    if (glxc->largeCmdRequestsSoFar == 0) {
         __GLXrenderSizeData entry;
         int extra = 0;
         int left = (req->length << 2) - sz_xGLXRenderLargeReq;
@@ -2193,21 +2203,21 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * pc)
         /*
          ** Make enough space in the buffer, then copy the entire request.
          */
-        if (cl->largeCmdBufSize < cmdlen) {
-	    GLbyte *newbuf = cl->largeCmdBuf;
+        if (glxc->largeCmdBufSize < cmdlen) {
+	    GLbyte *newbuf = glxc->largeCmdBuf;
 
 	    if (!(newbuf = realloc(newbuf, cmdlen)))
 		return BadAlloc;
 
-	    cl->largeCmdBuf = newbuf;
-            cl->largeCmdBufSize = cmdlen;
+	    glxc->largeCmdBuf = newbuf;
+            glxc->largeCmdBufSize = cmdlen;
         }
-        memcpy(cl->largeCmdBuf, pc, dataBytes);
+        memcpy(glxc->largeCmdBuf, pc, dataBytes);
 
-        cl->largeCmdBytesSoFar = dataBytes;
-        cl->largeCmdBytesTotal = cmdlen;
-        cl->largeCmdRequestsSoFar = 1;
-        cl->largeCmdRequestsTotal = req->requestTotal;
+        glxc->largeCmdBytesSoFar = dataBytes;
+        glxc->largeCmdBytesTotal = cmdlen;
+        glxc->largeCmdRequestsSoFar = 1;
+        glxc->largeCmdRequestsTotal = req->requestTotal;
         return Success;
 
     }
@@ -2221,37 +2231,37 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * pc)
         /*
          ** Check the request number and the total request count.
          */
-        if (req->requestNumber != cl->largeCmdRequestsSoFar + 1) {
+        if (req->requestNumber != glxc->largeCmdRequestsSoFar + 1) {
             client->errorValue = req->requestNumber;
-            __glXResetLargeCommandStatus(cl);
+            ResetLargeCommandStatus(glxc);
             return __glXError(GLXBadLargeRequest);
         }
-        if (req->requestTotal != cl->largeCmdRequestsTotal) {
+        if (req->requestTotal != glxc->largeCmdRequestsTotal) {
             client->errorValue = req->requestTotal;
-            __glXResetLargeCommandStatus(cl);
+            ResetLargeCommandStatus(glxc);
             return __glXError(GLXBadLargeRequest);
         }
 
         /*
          ** Check that we didn't get too much data.
          */
-        if ((bytesSoFar = safe_add(cl->largeCmdBytesSoFar, dataBytes)) < 0) {
+        if ((bytesSoFar = safe_add(glxc->largeCmdBytesSoFar, dataBytes)) < 0) {
             client->errorValue = dataBytes;
-            __glXResetLargeCommandStatus(cl);
+            ResetLargeCommandStatus(glxc);
             return __glXError(GLXBadLargeRequest);
         }
 
-        if (bytesSoFar > cl->largeCmdBytesTotal) {
+        if (bytesSoFar > glxc->largeCmdBytesTotal) {
             client->errorValue = dataBytes;
-            __glXResetLargeCommandStatus(cl);
+            ResetLargeCommandStatus(glxc);
             return __glXError(GLXBadLargeRequest);
         }
 
-        memcpy(cl->largeCmdBuf + cl->largeCmdBytesSoFar, pc, dataBytes);
-        cl->largeCmdBytesSoFar += dataBytes;
-        cl->largeCmdRequestsSoFar++;
+        memcpy(glxc->largeCmdBuf + glxc->largeCmdBytesSoFar, pc, dataBytes);
+        glxc->largeCmdBytesSoFar += dataBytes;
+        glxc->largeCmdRequestsSoFar++;
 
-        if (req->requestNumber == cl->largeCmdRequestsTotal) {
+        if (req->requestNumber == glxc->largeCmdRequestsTotal) {
             __GLXdispatchRenderProcPtr proc;
 
             /*
@@ -2267,12 +2277,12 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * pc)
              ** fixes a bug that did not allow large commands of odd sizes to
              ** be accepted by the server.
              */
-            if (safe_pad(cl->largeCmdBytesSoFar) != cl->largeCmdBytesTotal) {
+            if (safe_pad(glxc->largeCmdBytesSoFar) != glxc->largeCmdBytesTotal) {
                 client->errorValue = dataBytes;
-                __glXResetLargeCommandStatus(cl);
+                ResetLargeCommandStatus(glxc);
                 return __glXError(GLXBadLargeRequest);
             }
-            hdr = (__GLXrenderLargeHeader *) cl->largeCmdBuf;
+            hdr = (__GLXrenderLargeHeader *) glxc->largeCmdBuf;
             /*
              ** The opcode and length field in the header had already been
              ** swapped when the first request was received.
@@ -2292,12 +2302,12 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * pc)
             /*
              ** Skip over the header and execute the command.
              */
-            (*proc) (cl->largeCmdBuf + __GLX_RENDER_LARGE_HDR_SIZE);
+            (*proc) (glxc->largeCmdBuf + __GLX_RENDER_LARGE_HDR_SIZE);
 
             /*
              ** Reset for the next RenderLarge series.
              */
-            __glXResetLargeCommandStatus(cl);
+            ResetLargeCommandStatus(glxc);
         }
         else {
             /*
diff --git a/glx/glxcontext.h b/glx/glxcontext.h
index 19d347fdb..8f623b4b4 100644
--- a/glx/glxcontext.h
+++ b/glx/glxcontext.h
@@ -112,6 +112,16 @@ struct __GLXcontext {
     GLuint *selectBuf;
     GLint selectBufSize;        /* number of elements allocated */
 
+    /*
+     ** Keep track of large rendering commands, which span multiple requests.
+     */
+    GLint largeCmdBytesSoFar;   /* bytes received so far        */
+    GLint largeCmdBytesTotal;   /* total bytes expected         */
+    GLint largeCmdRequestsSoFar;        /* requests received so far     */
+    GLint largeCmdRequestsTotal;        /* total requests expected      */
+    GLbyte *largeCmdBuf;
+    GLint largeCmdBufSize;
+
     /*
      ** The drawable private this context is bound to
      */
diff --git a/glx/glxext.c b/glx/glxext.c
index 3852aa09b..90c3ff2eb 100644
--- a/glx/glxext.c
+++ b/glx/glxext.c
@@ -65,18 +65,6 @@ static DevPrivateKeyRec glxClientPrivateKeyRec;
 static int __glXDispatch(ClientPtr);
 static GLboolean __glXFreeContext(__GLXcontext * cx);
 
-/*
-** Reset state used to keep track of large (multi-request) commands.
-*/
-void
-__glXResetLargeCommandStatus(__GLXclientState * cl)
-{
-    cl->largeCmdBytesSoFar = 0;
-    cl->largeCmdBytesTotal = 0;
-    cl->largeCmdRequestsSoFar = 0;
-    cl->largeCmdRequestsTotal = 0;
-}
-
 /*
  * This procedure is called when the client who created the context goes away
  * OR when glXDestroyContext is called. If the context is current for a client
@@ -188,6 +176,7 @@ __glXFreeContext(__GLXcontext * cx)
 
     free(cx->feedbackBuf);
     free(cx->selectBuf);
+    free(cx->largeCmdBuf);
     if (cx == lastGLContext) {
         lastGLContext = NULL;
     }
@@ -270,7 +259,6 @@ glxClientCallback(CallbackListPtr *list, void *closure, void *data)
     switch (pClient->clientState) {
     case ClientStateGone:
         free(cl->returnBuf);
-        free(cl->largeCmdBuf);
         free(cl->GLClientextensions);
         cl->returnBuf = NULL;
         cl->GLClientextensions = NULL;
@@ -571,6 +559,9 @@ xorgGlxCreateVendor(void)
 __GLXcontext *
 __glXForceCurrent(__GLXclientState * cl, GLXContextTag tag, int *error)
 {
+    ClientPtr client = cl->client;
+    REQUEST(xGLXSingleReq);
+
     __GLXcontext *cx;
 
     /*
@@ -584,6 +575,13 @@ __glXForceCurrent(__GLXclientState * cl, GLXContextTag tag, int *error)
         return 0;
     }
 
+    /* If we're expecting a glXRenderLarge request, this better be one. */
+    if (cx->largeCmdRequestsSoFar != 0 && stuff->glxCode != X_GLXRenderLarge) {
+        client->errorValue = stuff->glxCode;
+        *error = __glXError(GLXBadLargeRequest);
+        return 0;
+    }
+
     if (!cx->isDirect) {
         if (cx->drawPriv == NULL) {
             /*
@@ -690,13 +688,6 @@ __glXDispatch(ClientPtr client)
     opcode = stuff->glxCode;
     cl = glxGetClient(client);
 
-    /*
-     ** If we're expecting a glXRenderLarge request, this better be one.
-     */
-    if ((cl->largeCmdRequestsSoFar != 0) && (opcode != X_GLXRenderLarge)) {
-        client->errorValue = stuff->glxCode;
-        return __glXError(GLXBadLargeRequest);
-    }
 
     if (!cl->client)
         cl->client = client;
diff --git a/glx/glxext.h b/glx/glxext.h
index af59165bf..5ad497e4a 100644
--- a/glx/glxext.h
+++ b/glx/glxext.h
@@ -57,7 +57,6 @@ extern Bool __glXAddContext(__GLXcontext * cx);
 extern void __glXErrorCallBack(GLenum code);
 extern void __glXClearErrorOccured(void);
 extern GLboolean __glXErrorOccured(void);
-extern void __glXResetLargeCommandStatus(__GLXclientState *);
 
 extern const char GLServerVersion[];
 extern int DoGetString(__GLXclientState * cl, GLbyte * pc, GLboolean need_swap);
diff --git a/glx/glxserver.h b/glx/glxserver.h
index 8ffde2342..60bdeb00d 100644
--- a/glx/glxserver.h
+++ b/glx/glxserver.h
@@ -115,16 +115,6 @@ struct __GLXclientStateRec {
     GLbyte *returnBuf;
     GLint returnBufSize;
 
-    /*
-     ** Keep track of large rendering commands, which span multiple requests.
-     */
-    GLint largeCmdBytesSoFar;   /* bytes received so far        */
-    GLint largeCmdBytesTotal;   /* total bytes expected         */
-    GLint largeCmdRequestsSoFar;        /* requests received so far     */
-    GLint largeCmdRequestsTotal;        /* total requests expected      */
-    GLbyte *largeCmdBuf;
-    GLint largeCmdBufSize;
-
     /* Back pointer to X client record */
     ClientPtr client;
 
-- 
2.14.3



More information about the xorg-devel mailing list