From 5c09c89903814a0f9d8dd02faabe762c19d51a61 Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Fri, 12 Jul 2024 14:52:51 +0200 Subject: [PATCH] xfixes: canonicalize reply structures Canonicalize all reply structs onto stack allocation and static initialization, like already done in most other extension. So make the code easier to understand and allow further simplifications by subsequent commits (we can then use generic macros for doing the actual sending, as well as byteorder swapping, size computation, etc), Also gaining a little bit efficiency by skipping some heap allocations. Dynamically sized payload buffers (where the upper bound isn't known), are still allocated on heap. Signed-off-by: Enrico Weigelt, metux IT consult --- xfixes/cursor.c | 130 ++++++++++++++++++++++---------------------- xfixes/disconnect.c | 9 ++- xfixes/region.c | 44 +++++++-------- 3 files changed, 91 insertions(+), 92 deletions(-) diff --git a/xfixes/cursor.c b/xfixes/cursor.c index 3c10c33fa..0fd41dca7 100644 --- a/xfixes/cursor.c +++ b/xfixes/cursor.c @@ -355,7 +355,6 @@ ProcXFixesGetCursorImage(ClientPtr client) { /* REQUEST(xXFixesGetCursorImageReq); */ CursorPtr pCursor; - CARD32 *image; int npixels, width, height, rc, x, y; REQUEST_SIZE_MATCH(xXFixesGetCursorImageReq); @@ -371,39 +370,40 @@ ProcXFixesGetCursorImage(ClientPtr client) height = pCursor->bits->height; npixels = width * height; - xXFixesGetCursorImageReply *rep = calloc(1, - sizeof(xXFixesGetCursorImageReply) + npixels * sizeof(CARD32)); - if (!rep) + CARD32 *image = calloc(npixels, sizeof(CARD32)); + if (!image) return BadAlloc; - rep->type = X_Reply; - rep->sequenceNumber = client->sequence; - rep->length = npixels; - rep->width = width; - rep->height = height; - rep->x = x; - rep->y = y; - rep->xhot = pCursor->bits->xhot; - rep->yhot = pCursor->bits->yhot; - rep->cursorSerial = pCursor->serialNumber; - - image = (CARD32 *) (rep + 1); CopyCursorToImage(pCursor, image); + + xXFixesGetCursorImageReply rep = { + .type = X_Reply, + .sequenceNumber = client->sequence, + .length = npixels, + .width = width, + .height = height, + .x = x, + .y = y, + .xhot = pCursor->bits->xhot, + .yhot = pCursor->bits->yhot, + .cursorSerial = pCursor->serialNumber, + }; + if (client->swapped) { - swaps(&rep->sequenceNumber); - swapl(&rep->length); - swaps(&rep->x); - swaps(&rep->y); - swaps(&rep->width); - swaps(&rep->height); - swaps(&rep->xhot); - swaps(&rep->yhot); - swapl(&rep->cursorSerial); + swaps(&rep.sequenceNumber); + swapl(&rep.length); + swaps(&rep.x); + swaps(&rep.y); + swaps(&rep.width); + swaps(&rep.height); + swaps(&rep.xhot); + swaps(&rep.yhot); + swapl(&rep.cursorSerial); SwapLongs(image, npixels); } - WriteToClient(client, - sizeof(xXFixesGetCursorImageReply) + (npixels << 2), rep); - free(rep); + WriteToClient(client, sizeof(rep), &rep); + WriteToClient(client, npixels * sizeof(CARD32), image); + free(image); return Success; } @@ -440,9 +440,9 @@ SProcXFixesSetCursorName(ClientPtr client) int ProcXFixesGetCursorName(ClientPtr client) { - CursorPtr pCursor; - REQUEST(xXFixesGetCursorNameReq); + + CursorPtr pCursor; const char *str; int len; @@ -487,10 +487,9 @@ ProcXFixesGetCursorImageAndName(ClientPtr client) { /* REQUEST(xXFixesGetCursorImageAndNameReq); */ CursorPtr pCursor; - CARD32 *image; int npixels; const char *name; - int nbytes, nbytesRound; + int nbytes; int width, height; int rc, x, y; @@ -508,47 +507,48 @@ ProcXFixesGetCursorImageAndName(ClientPtr client) npixels = width * height; name = pCursor->name ? NameForAtom(pCursor->name) : ""; nbytes = strlen(name); - nbytesRound = pad_to_int32(nbytes); - xXFixesGetCursorImageAndNameReply *rep = calloc(1, - sizeof(xXFixesGetCursorImageAndNameReply) + - npixels * sizeof(CARD32) + nbytesRound); - if (!rep) + // pixmap plus name (padded to 4 bytes) + const size_t image_size = (npixels + bytes_to_int32(nbytes)) * sizeof(CARD32); + CARD32 *image = calloc(1, image_size); + if (!image) return BadAlloc; - rep->type = X_Reply; - rep->sequenceNumber = client->sequence; - rep->length = npixels + bytes_to_int32(nbytesRound); - rep->width = width; - rep->height = height; - rep->x = x; - rep->y = y; - rep->xhot = pCursor->bits->xhot; - rep->yhot = pCursor->bits->yhot; - rep->cursorSerial = pCursor->serialNumber; - rep->cursorName = pCursor->name; - rep->nbytes = nbytes; - - image = (CARD32 *) (rep + 1); CopyCursorToImage(pCursor, image); memcpy((image + npixels), name, nbytes); + + xXFixesGetCursorImageAndNameReply rep = { + .type = X_Reply, + .sequenceNumber = client->sequence, + .length = bytes_to_int32(image_size), + .width = width, + .height = height, + .x = x, + .y = y, + .xhot = pCursor->bits->xhot, + .yhot = pCursor->bits->yhot, + .cursorSerial = pCursor->serialNumber, + .cursorName = pCursor->name, + .nbytes = nbytes, + }; + if (client->swapped) { - swaps(&rep->sequenceNumber); - swapl(&rep->length); - swaps(&rep->x); - swaps(&rep->y); - swaps(&rep->width); - swaps(&rep->height); - swaps(&rep->xhot); - swaps(&rep->yhot); - swapl(&rep->cursorSerial); - swapl(&rep->cursorName); - swaps(&rep->nbytes); + swaps(&rep.sequenceNumber); + swapl(&rep.length); + swaps(&rep.x); + swaps(&rep.y); + swaps(&rep.width); + swaps(&rep.height); + swaps(&rep.xhot); + swaps(&rep.yhot); + swapl(&rep.cursorSerial); + swapl(&rep.cursorName); + swaps(&rep.nbytes); SwapLongs(image, npixels); } - WriteToClient(client, sizeof(xXFixesGetCursorImageAndNameReply) + - (npixels << 2) + nbytesRound, rep); - free(rep); + WriteToClient(client, sizeof(rep), &rep); + WriteToClient(client, image_size, image); + free(image); return Success; } diff --git a/xfixes/disconnect.c b/xfixes/disconnect.c index 15ea2082b..8d52b50aa 100644 --- a/xfixes/disconnect.c +++ b/xfixes/disconnect.c @@ -87,21 +87,20 @@ int ProcXFixesGetClientDisconnectMode(ClientPtr client) { ClientDisconnectPtr pDisconnect = GetClientDisconnect(client); - xXFixesGetClientDisconnectModeReply reply; REQUEST_SIZE_MATCH(xXFixesGetClientDisconnectModeReq); - reply = (xXFixesGetClientDisconnectModeReply) { + xXFixesGetClientDisconnectModeReply rep = { .type = X_Reply, .sequenceNumber = client->sequence, .length = 0, .disconnect_mode = pDisconnect->disconnect_mode, }; if (client->swapped) { - swaps(&reply.sequenceNumber); - swapl(&reply.disconnect_mode); + swaps(&rep.sequenceNumber); + swapl(&rep.disconnect_mode); } - WriteToClient(client, sizeof(xXFixesGetClientDisconnectModeReply), &reply); + WriteToClient(client, sizeof(rep), &rep); return Success; } diff --git a/xfixes/region.c b/xfixes/region.c index 2156c3fd7..5928ce5f3 100644 --- a/xfixes/region.c +++ b/xfixes/region.c @@ -509,8 +509,6 @@ int ProcXFixesFetchRegion(ClientPtr client) { RegionPtr pRegion; - xXFixesFetchRegionReply *reply; - xRectangle *pRect; BoxPtr pExtent; BoxPtr pBox; int i, nBox; @@ -524,37 +522,39 @@ ProcXFixesFetchRegion(ClientPtr client) pBox = RegionRects(pRegion); nBox = RegionNumRects(pRegion); - reply = calloc(sizeof(xXFixesFetchRegionReply) + nBox * sizeof(xRectangle), - 1); - if (!reply) + xRectangle *pRect = calloc(nBox, sizeof(xRectangle)); + if (!pRect) return BadAlloc; - reply->type = X_Reply; - reply->sequenceNumber = client->sequence; - reply->length = nBox << 1; - reply->x = pExtent->x1; - reply->y = pExtent->y1; - reply->width = pExtent->x2 - pExtent->x1; - reply->height = pExtent->y2 - pExtent->y1; - pRect = (xRectangle *) (reply + 1); for (i = 0; i < nBox; i++) { pRect[i].x = pBox[i].x1; pRect[i].y = pBox[i].y1; pRect[i].width = pBox[i].x2 - pBox[i].x1; pRect[i].height = pBox[i].y2 - pBox[i].y1; } + + xXFixesFetchRegionReply rep = { + .type = X_Reply, + .sequenceNumber = client->sequence, + .length = nBox << 1, + .x = pExtent->x1, + .y = pExtent->y1, + .width = pExtent->x2 - pExtent->x1, + .height = pExtent->y2 - pExtent->y1, + }; + if (client->swapped) { - swaps(&reply->sequenceNumber); - swapl(&reply->length); - swaps(&reply->x); - swaps(&reply->y); - swaps(&reply->width); - swaps(&reply->height); + swaps(&rep.sequenceNumber); + swapl(&rep.length); + swaps(&rep.x); + swaps(&rep.y); + swaps(&rep.width); + swaps(&rep.height); SwapShorts((INT16 *) pRect, nBox * 4); } - WriteToClient(client, sizeof(xXFixesFetchRegionReply) + - nBox * sizeof(xRectangle), (char *) reply); - free(reply); + WriteToClient(client, sizeof(rep), &rep); + WriteToClient(client, nBox * sizeof(xRectangle), pRect); + free(pRect); return Success; }