[PATCH v2 06/29] Use temporary variables instead of parts of reply structures

Alan Coopersmith alan.coopersmith at oracle.com
Wed Jul 4 15:37:20 PDT 2012


When passing variable pointers to functions or otherwise doing long
sequences to compute values for replies, create & use some new
temporary variables, to allow for simpler initialization of reply
structures in the following patches.

Move memsets & other initializations to group with the rest of the
filling in of the reply structure, now that they're not needed so
early in the code path.

Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
---
 Xext/shm.c                    |    6 ++++--
 dix/dispatch.c                |   18 ++++++++++++------
 dix/events.c                  |   15 ++++++++++-----
 hw/kdrive/ephyr/ephyrdriext.c |   14 ++++++++------
 hw/kdrive/ephyr/ephyrglxext.c |    4 +++-
 hw/xfree86/dri/xf86dri.c      |   12 +++++++-----
 randr/rrcrtc.c                |   15 +++++++++------
 randr/rrscreen.c              |   19 ++++++++++---------
 randr/rrxinerama.c            |   11 +++++++----
 record/record.c               |   11 +++++++----
 10 files changed, 77 insertions(+), 48 deletions(-)

diff --git a/Xext/shm.c b/Xext/shm.c
index aa3feae..a732170 100644
--- a/Xext/shm.c
+++ b/Xext/shm.c
@@ -616,6 +616,7 @@ ProcShmGetImage(ClientPtr client)
     Mask plane = 0;
     xShmGetImageReply xgi;
     ShmDescPtr shmdesc;
+    VisualID visual = None;
     int rc;
 
     REQUEST(xShmGetImageReq);
@@ -646,18 +647,19 @@ ProcShmGetImage(ClientPtr client)
                stuff->y + (int) stuff->height >
                wBorderWidth((WindowPtr) pDraw) + (int) pDraw->height)
             return BadMatch;
-        xgi.visual = wVisual(((WindowPtr) pDraw));
+        visual = wVisual(((WindowPtr) pDraw));
     }
     else {
         if (stuff->x < 0 ||
             stuff->x + (int) stuff->width > pDraw->width ||
             stuff->y < 0 || stuff->y + (int) stuff->height > pDraw->height)
             return BadMatch;
-        xgi.visual = None;
+        visual = None;
     }
     xgi.type = X_Reply;
     xgi.length = 0;
     xgi.sequenceNumber = client->sequence;
+    xgi.visual = visual;
     xgi.depth = pDraw->depth;
     if (stuff->format == ZPixmap) {
         length = PixmapBytePad(stuff->width, pDraw->depth) * stuff->height;
diff --git a/dix/dispatch.c b/dix/dispatch.c
index 0969d8e..54ab6bb 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -2801,17 +2801,21 @@ ProcLookupColor(ClientPtr client)
     rc = dixLookupResourceByType((pointer *) &pcmp, stuff->cmap, RT_COLORMAP,
                                  client, DixReadAccess);
     if (rc == Success) {
-        xLookupColorReply lcr;
+        CARD16 exactRed, exactGreen, exactBlue;
 
         if (OsLookupColor
             (pcmp->pScreen->myNum, (char *) &stuff[1], stuff->nbytes,
-             &lcr.exactRed, &lcr.exactGreen, &lcr.exactBlue)) {
+             &exactRed, &exactGreen, &exactBlue)) {
+            xLookupColorReply lcr;
             lcr.type = X_Reply;
             lcr.length = 0;
             lcr.sequenceNumber = client->sequence;
-            lcr.screenRed = lcr.exactRed;
-            lcr.screenGreen = lcr.exactGreen;
-            lcr.screenBlue = lcr.exactBlue;
+            lcr.exactRed = exactRed;
+            lcr.exactGreen = exactGreen;
+            lcr.exactBlue = exactBlue;
+            lcr.screenRed = exactRed;
+            lcr.screenGreen = exactGreen;
+            lcr.screenBlue = exactBlue;
             (*pcmp->pScreen->ResolveColor) (&lcr.screenRed,
                                             &lcr.screenGreen,
                                             &lcr.screenBlue, pcmp->pVisual);
@@ -3109,6 +3113,7 @@ ProcListHosts(ClientPtr client)
 {
     xListHostsReply reply;
     int len, nHosts, result;
+    BOOL enabled;
     pointer pdata;
 
     /* REQUEST(xListHostsReq); */
@@ -3120,10 +3125,11 @@ ProcListHosts(ClientPtr client)
     if (result != Success)
         return result;
 
-    result = GetHosts(&pdata, &nHosts, &len, &reply.enabled);
+    result = GetHosts(&pdata, &nHosts, &len, &enabled);
     if (result != Success)
         return result;
     reply.type = X_Reply;
+    reply.enabled = enabled;
     reply.sequenceNumber = client->sequence;
     reply.nHosts = nHosts;
     reply.length = bytes_to_int32(len);
diff --git a/dix/events.c b/dix/events.c
index 6e4385a..c5eceec 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -4797,6 +4797,7 @@ ProcGrabPointer(ClientPtr client)
     GrabMask mask;
     WindowPtr confineTo;
     CursorPtr oldCursor;
+    BYTE status;
 
     REQUEST(xGrabPointerReq);
     int rc;
@@ -4818,7 +4819,6 @@ ProcGrabPointer(ClientPtr client)
             return rc;
     }
 
-    memset(&rep, 0, sizeof(xGrabPointerReply));
     oldCursor = NullCursor;
     grab = device->deviceGrab.grab;
 
@@ -4833,14 +4833,16 @@ ProcGrabPointer(ClientPtr client)
 
     rc = GrabDevice(client, device, stuff->pointerMode, stuff->keyboardMode,
                     stuff->grabWindow, stuff->ownerEvents, stuff->time,
-                    &mask, CORE, stuff->cursor, stuff->confineTo, &rep.status);
+                    &mask, CORE, stuff->cursor, stuff->confineTo, &status);
     if (rc != Success)
         return rc;
 
-    if (oldCursor && rep.status == GrabSuccess)
+    if (oldCursor && status == GrabSuccess)
         FreeCursor(oldCursor, (Cursor) 0);
 
+    memset(&rep, 0, sizeof(xGrabPointerReply));
     rep.type = X_Reply;
+    rep.status = status;
     rep.sequenceNumber = client->sequence;
     rep.length = 0;
     WriteReplyToClient(client, sizeof(xGrabPointerReply), &rep);
@@ -5059,6 +5061,7 @@ int
 ProcGrabKeyboard(ClientPtr client)
 {
     xGrabKeyboardReply rep;
+    BYTE status;
 
     REQUEST(xGrabKeyboardReq);
     int result;
@@ -5067,17 +5070,19 @@ ProcGrabKeyboard(ClientPtr client)
 
     REQUEST_SIZE_MATCH(xGrabKeyboardReq);
 
-    memset(&rep, 0, sizeof(xGrabKeyboardReply));
     mask.core = KeyPressMask | KeyReleaseMask;
 
     result = GrabDevice(client, keyboard, stuff->pointerMode,
                         stuff->keyboardMode, stuff->grabWindow,
                         stuff->ownerEvents, stuff->time, &mask, CORE, None,
-                        None, &rep.status);
+                        None, &status);
 
     if (result != Success)
         return result;
+
+    memset(&rep, 0, sizeof(xGrabKeyboardReply));
     rep.type = X_Reply;
+    rep.status = status;
     rep.sequenceNumber = client->sequence;
     rep.length = 0;
     WriteReplyToClient(client, sizeof(xGrabKeyboardReply), &rep);
diff --git a/hw/kdrive/ephyr/ephyrdriext.c b/hw/kdrive/ephyr/ephyrdriext.c
index aefbcfb..65e0ff8 100644
--- a/hw/kdrive/ephyr/ephyrdriext.c
+++ b/hw/kdrive/ephyr/ephyrdriext.c
@@ -586,6 +586,7 @@ ProcXF86DRIOpenConnection(register ClientPtr client)
     xXF86DRIOpenConnectionReply rep;
     drm_handle_t hSAREA;
     char *busIdString = NULL;
+    CARD32 busIdStringLength = 0;
 
     REQUEST(xXF86DRIOpenConnectionReq);
     REQUEST_SIZE_MATCH(xXF86DRIOpenConnectionReq);
@@ -600,15 +601,16 @@ ProcXF86DRIOpenConnection(register ClientPtr client)
         return BadValue;
     }
 
+    if (busIdString)
+        busIdStringLength = strlen(busIdString);
+
     rep.type = X_Reply;
     rep.sequenceNumber = client->sequence;
-    rep.busIdStringLength = 0;
-    if (busIdString)
-        rep.busIdStringLength = strlen(busIdString);
+    rep.busIdStringLength = busIdStringLength;
     rep.length =
         bytes_to_int32(SIZEOF(xXF86DRIOpenConnectionReply) -
                        SIZEOF(xGenericReply) +
-                       pad_to_int32(rep.busIdStringLength));
+                       pad_to_int32(busIdStringLength));
 
     rep.hSAREALow = (CARD32) (hSAREA & 0xffffffff);
 #if defined(LONG64) && !defined(__linux__)
@@ -618,8 +620,8 @@ ProcXF86DRIOpenConnection(register ClientPtr client)
 #endif
 
     WriteToClient(client, sizeof(xXF86DRIOpenConnectionReply), &rep);
-    if (rep.busIdStringLength)
-        WriteToClient(client, rep.busIdStringLength, busIdString);
+    if (busIdStringLength)
+        WriteToClient(client, busIdStringLength, busIdString);
     free(busIdString);
     EPHYR_LOG("leave\n");
     return Success;
diff --git a/hw/kdrive/ephyr/ephyrglxext.c b/hw/kdrive/ephyr/ephyrglxext.c
index 1287e04..f5c4c18 100644
--- a/hw/kdrive/ephyr/ephyrglxext.c
+++ b/hw/kdrive/ephyr/ephyrglxext.c
@@ -512,6 +512,7 @@ ephyrGLXMakeCurrentReal(__GLXclientState * a_cl, GLbyte * a_pc, Bool a_do_swap)
     xGLXMakeCurrentReq *req = (xGLXMakeCurrentReq *) a_pc;
     xGLXMakeCurrentReply reply;
     DrawablePtr drawable = NULL;
+    GLXContextTag contextTag = 0;
     int rc = 0;
 
     EPHYR_LOG("enter\n");
@@ -525,13 +526,14 @@ ephyrGLXMakeCurrentReal(__GLXclientState * a_cl, GLbyte * a_pc, Bool a_do_swap)
     if (!ephyrHostGLXMakeCurrent(hostx_get_window(drawable->pScreen->myNum),
                                  req->context,
                                  req->oldContextTag,
-                                 (int *) &reply.contextTag)) {
+                                 (int *) &contextTag)) {
         EPHYR_LOG_ERROR("ephyrHostGLXMakeCurrent() failed\n");
         goto out;
     }
     reply.length = 0;
     reply.type = X_Reply;
     reply.sequenceNumber = a_cl->client->sequence;
+    reply.contextTag = contextTag;
     if (a_do_swap) {
         __GLX_DECLARE_SWAP_VARIABLES;
         __GLX_SWAP_SHORT(&reply.sequenceNumber);
diff --git a/hw/xfree86/dri/xf86dri.c b/hw/xfree86/dri/xf86dri.c
index ee7b213..43504b7 100644
--- a/hw/xfree86/dri/xf86dri.c
+++ b/hw/xfree86/dri/xf86dri.c
@@ -141,6 +141,7 @@ ProcXF86DRIOpenConnection(register ClientPtr client)
     xXF86DRIOpenConnectionReply rep;
     drm_handle_t hSAREA;
     char *busIdString;
+    CARD32 busIdStringLength = 0;
 
     REQUEST(xXF86DRIOpenConnectionReq);
     REQUEST_SIZE_MATCH(xXF86DRIOpenConnectionReq);
@@ -154,11 +155,12 @@ ProcXF86DRIOpenConnection(register ClientPtr client)
         return BadValue;
     }
 
+    if (busIdString)
+        busIdStringLength = strlen(busIdString);
+
     rep.type = X_Reply;
     rep.sequenceNumber = client->sequence;
-    rep.busIdStringLength = 0;
-    if (busIdString)
-        rep.busIdStringLength = strlen(busIdString);
+    rep.busIdStringLength = busIdStringLength;
     rep.length =
         bytes_to_int32(SIZEOF(xXF86DRIOpenConnectionReply) -
                        SIZEOF(xGenericReply) +
@@ -172,8 +174,8 @@ ProcXF86DRIOpenConnection(register ClientPtr client)
 #endif
 
     WriteToClient(client, sizeof(xXF86DRIOpenConnectionReply), &rep);
-    if (rep.busIdStringLength)
-        WriteToClient(client, rep.busIdStringLength, busIdString);
+    if (busIdStringLength)
+        WriteToClient(client, busIdStringLength, busIdString);
     return Success;
 }
 
diff --git a/randr/rrcrtc.c b/randr/rrcrtc.c
index a528c47..1657641 100644
--- a/randr/rrcrtc.c
+++ b/randr/rrcrtc.c
@@ -828,6 +828,7 @@ ProcRRSetCrtcConfig(ClientPtr client)
     TimeStamp time;
     Rotation rotation;
     int ret, i, j;
+    CARD8 status;
 
     REQUEST_AT_LEAST_SIZE(xRRSetCrtcConfigReq);
     numOutputs = (stuff->length - bytes_to_int32(SIZEOF(xRRSetCrtcConfigReq)));
@@ -906,7 +907,7 @@ ProcRRSetCrtcConfig(ClientPtr client)
 
     if (!pScrPriv) {
         time = currentTime;
-        rep.status = RRSetConfigFailed;
+        status = RRSetConfigFailed;
         goto sendReply;
     }
 
@@ -980,17 +981,17 @@ ProcRRSetCrtcConfig(ClientPtr client)
 
     if (!RRCrtcSet(crtc, mode, stuff->x, stuff->y,
                    rotation, numOutputs, outputs)) {
-        rep.status = RRSetConfigFailed;
+        status = RRSetConfigFailed;
         goto sendReply;
     }
-    rep.status = RRSetConfigSuccess;
+    status = RRSetConfigSuccess;
     pScrPriv->lastSetTime = time;
 
  sendReply:
     free(outputs);
 
     rep.type = X_Reply;
-    /* rep.status has already been filled in */
+    rep.status = status;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
     rep.newTimestamp = pScrPriv->lastSetTime.milliseconds;
@@ -1085,6 +1086,7 @@ ProcRRSetPanning(ClientPtr client)
     BoxRec total;
     BoxRec tracking;
     INT16 border[4];
+    CARD8 status;
 
     REQUEST_SIZE_MATCH(xRRSetPanningReq);
     VERIFY_RR_CRTC(stuff->crtc, crtc, DixReadAccess);
@@ -1097,7 +1099,7 @@ ProcRRSetPanning(ClientPtr client)
 
     if (!pScrPriv) {
         time = currentTime;
-        rep.status = RRSetConfigFailed;
+        status = RRSetConfigFailed;
         goto sendReply;
     }
 
@@ -1124,10 +1126,11 @@ ProcRRSetPanning(ClientPtr client)
 
     pScrPriv->lastSetTime = time;
 
-    rep.status = RRSetConfigSuccess;
+    status = RRSetConfigSuccess;
 
  sendReply:
     rep.type = X_Reply;
+    rep.status = status;
     rep.sequenceNumber = client->sequence;
     rep.length = 0;
     rep.newTimestamp = pScrPriv->lastSetTime.milliseconds;
diff --git a/randr/rrscreen.c b/randr/rrscreen.c
index 50b1988..0ec608c 100644
--- a/randr/rrscreen.c
+++ b/randr/rrscreen.c
@@ -713,6 +713,7 @@ ProcRRSetScreenConfig(ClientPtr client)
     Rotation rotation;
     int rate;
     Bool has_rate;
+    CARD8 status;
     RROutputPtr output;
     RRCrtcPtr crtc;
     RRModePtr mode;
@@ -743,7 +744,7 @@ ProcRRSetScreenConfig(ClientPtr client)
 
     if (!pScrPriv) {
         time = currentTime;
-        rep.status = RRSetConfigFailed;
+        status = RRSetConfigFailed;
         goto sendReply;
     }
     if (!RRGetInfo(pScreen, FALSE))
@@ -752,7 +753,7 @@ ProcRRSetScreenConfig(ClientPtr client)
     output = RRFirstOutput(pScreen);
     if (!output) {
         time = currentTime;
-        rep.status = RRSetConfigFailed;
+        status = RRSetConfigFailed;
         goto sendReply;
     }
 
@@ -768,7 +769,7 @@ ProcRRSetScreenConfig(ClientPtr client)
      * stop working after several hours have passed (freedesktop bug #6502).
      */
     if (stuff->configTimestamp != pScrPriv->lastConfigTime.milliseconds) {
-        rep.status = RRSetConfigInvalidConfigTime;
+        status = RRSetConfigInvalidConfigTime;
         goto sendReply;
     }
 
@@ -847,7 +848,7 @@ ProcRRSetScreenConfig(ClientPtr client)
      * the last set-time
      */
     if (CompareTimeStamps(time, pScrPriv->lastSetTime) < 0) {
-        rep.status = RRSetConfigInvalidTime;
+        status = RRSetConfigInvalidTime;
         goto sendReply;
     }
 
@@ -879,24 +880,24 @@ ProcRRSetScreenConfig(ClientPtr client)
         for (c = 0; c < pScrPriv->numCrtcs; c++) {
             if (!RRCrtcSet(pScrPriv->crtcs[c], NULL, 0, 0, RR_Rotate_0,
                            0, NULL)) {
-                rep.status = RRSetConfigFailed;
+                status = RRSetConfigFailed;
                 /* XXX recover from failure */
                 goto sendReply;
             }
         }
         if (!RRScreenSizeSet(pScreen, width, height,
                              pScreen->mmWidth, pScreen->mmHeight)) {
-            rep.status = RRSetConfigFailed;
+            status = RRSetConfigFailed;
             /* XXX recover from failure */
             goto sendReply;
         }
     }
 
     if (!RRCrtcSet(crtc, mode, 0, 0, stuff->rotation, 1, &output))
-        rep.status = RRSetConfigFailed;
+        status = RRSetConfigFailed;
     else {
         pScrPriv->lastSetTime = time;
-        rep.status = RRSetConfigSuccess;
+        status = RRSetConfigSuccess;
     }
 
     /*
@@ -908,7 +909,7 @@ ProcRRSetScreenConfig(ClientPtr client)
     free(pData);
 
     rep.type = X_Reply;
-    /* rep.status has already been filled in */
+    rep.status = status;
     rep.length = 0;
     rep.sequenceNumber = client->sequence;
 
diff --git a/randr/rrxinerama.c b/randr/rrxinerama.c
index 269a63f..da3942f 100644
--- a/randr/rrxinerama.c
+++ b/randr/rrxinerama.c
@@ -299,16 +299,19 @@ ProcRRXineramaQueryScreens(ClientPtr client)
 {
     xXineramaQueryScreensReply rep;
     ScreenPtr pScreen = screenInfo.screens[RR_XINERAMA_SCREEN];
+    int n = 0;
 
     REQUEST_SIZE_MATCH(xXineramaQueryScreensReq);
 
-    if (RRXineramaScreenActive(pScreen))
+    if (RRXineramaScreenActive(pScreen)) {
         RRGetInfo(pScreen, FALSE);
+        n = RRXineramaScreenCount(pScreen);
+    }
 
     rep.type = X_Reply;
     rep.sequenceNumber = client->sequence;
-    rep.number = RRXineramaScreenCount(pScreen);
-    rep.length = bytes_to_int32(rep.number * sz_XineramaScreenInfo);
+    rep.number = n;
+    rep.length = bytes_to_int32(n * sz_XineramaScreenInfo);
     if (client->swapped) {
         swaps(&rep.sequenceNumber);
         swapl(&rep.length);
@@ -316,7 +319,7 @@ ProcRRXineramaQueryScreens(ClientPtr client)
     }
     WriteToClient(client, sizeof(xXineramaQueryScreensReply), &rep);
 
-    if (rep.number) {
+    if (n) {
         rrScrPriv(pScreen);
         int i;
         int has_primary = 0;
diff --git a/record/record.c b/record/record.c
index a3ee4dd..54a0e68 100644
--- a/record/record.c
+++ b/record/record.c
@@ -2135,6 +2135,7 @@ ProcRecordGetContext(ClientPtr client)
     GetContextRangeInfoPtr pri;
     int i;
     int err;
+    CARD32 nClients, length;
 
     REQUEST_SIZE_MATCH(xRecordGetContextReq);
     VERIFY_CONTEXT(pContext, stuff->context, client);
@@ -2218,12 +2219,12 @@ ProcRecordGetContext(ClientPtr client)
 
     /* calculate number of clients and reply length */
 
-    rep.nClients = 0;
-    rep.length = 0;
+    nClients = 0;
+    length = 0;
     for (pRCAP = pContext->pListOfRCAP, pri = pRangeInfo;
          pRCAP; pRCAP = pRCAP->pNextRCAP, pri++) {
-        rep.nClients += pRCAP->numClients;
-        rep.length += pRCAP->numClients *
+        nClients += pRCAP->numClients;
+        length += pRCAP->numClients *
             (bytes_to_int32(sizeof(xRecordClientInfo)) +
              pri->nRanges * bytes_to_int32(sizeof(xRecordRange)));
     }
@@ -2232,8 +2233,10 @@ ProcRecordGetContext(ClientPtr client)
 
     rep.type = X_Reply;
     rep.sequenceNumber = client->sequence;
+    rep.length = length;
     rep.enabled = pContext->pRecordingClient != NULL;
     rep.elementHeader = pContext->elemHeaders;
+    rep.nClients = nClients;
     if (client->swapped) {
         swaps(&rep.sequenceNumber);
         swapl(&rep.length);
-- 
1.7.9.2



More information about the xorg-devel mailing list