From 9de45d73ee4e4d48991c8033976d00f6cfd0f0d1 Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Fri, 12 Jul 2024 14:52:51 +0200 Subject: [PATCH] (submit/cleanup-xfixes) xfixes: use stack allocation and static init for reply structs Canonicalize all reply structures 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. Also gaining a little bit efficiency by skipping some heap allocations. Signed-off-by: Enrico Weigelt, metux IT consult --- xfixes/cursor.c | 141 +++++++++++++++++++++----------------------- xfixes/disconnect.c | 9 ++- xfixes/region.c | 43 +++++++------- 3 files changed, 91 insertions(+), 102 deletions(-) diff --git a/xfixes/cursor.c b/xfixes/cursor.c index 2e53334972..ddd208364c 100644 --- a/xfixes/cursor.c +++ b/xfixes/cursor.c @@ -364,9 +364,7 @@ int ProcXFixesGetCursorImage(ClientPtr client) { /* REQUEST(xXFixesGetCursorImageReq); */ - xXFixesGetCursorImageReply *rep; CursorPtr pCursor; - CARD32 *image; int npixels, width, height, rc, x, y; REQUEST_SIZE_MATCH(xXFixesGetCursorImageReq); @@ -381,39 +379,37 @@ ProcXFixesGetCursorImage(ClientPtr client) width = pCursor->bits->width; height = pCursor->bits->height; npixels = width * height; - rep = calloc(1, - sizeof(xXFixesGetCursorImageReply) + npixels * sizeof(CARD32)); - if (!rep) - 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); + CARD32 image[npixels]; 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(xXFixesGetCursorImageReply), &rep); + WriteToClient(client, sizeof(image), image); return Success; } @@ -460,10 +456,9 @@ SProcXFixesSetCursorName(ClientPtr client) int ProcXFixesGetCursorName(ClientPtr client) { - CursorPtr pCursor; - xXFixesGetCursorNameReply reply; - REQUEST(xXFixesGetCursorNameReq); + + CursorPtr pCursor; const char *str; int len; @@ -475,7 +470,7 @@ ProcXFixesGetCursorName(ClientPtr client) str = ""; len = strlen(str); - reply = (xXFixesGetCursorNameReply) { + xXFixesGetCursorNameReply rep = { .type = X_Reply, .sequenceNumber = client->sequence, .length = bytes_to_int32(len), @@ -483,12 +478,12 @@ ProcXFixesGetCursorName(ClientPtr client) .nbytes = len }; if (client->swapped) { - swaps(&reply.sequenceNumber); - swapl(&reply.length); - swapl(&reply.atom); - swaps(&reply.nbytes); + swaps(&rep.sequenceNumber); + swapl(&rep.length); + swapl(&rep.atom); + swaps(&rep.nbytes); } - WriteReplyToClient(client, sizeof(xXFixesGetCursorNameReply), &reply); + WriteReplyToClient(client, sizeof(xXFixesGetCursorNameReply), &rep); WriteToClient(client, len, str); return Success; @@ -509,12 +504,10 @@ int ProcXFixesGetCursorImageAndName(ClientPtr client) { /* REQUEST(xXFixesGetCursorImageAndNameReq); */ - xXFixesGetCursorImageAndNameReply *rep; CursorPtr pCursor; - CARD32 *image; int npixels; const char *name; - int nbytes, nbytesRound; + int nbytes; int width, height; int rc, x, y; @@ -532,45 +525,45 @@ ProcXFixesGetCursorImageAndName(ClientPtr client) npixels = width * height; name = pCursor->name ? NameForAtom(pCursor->name) : ""; nbytes = strlen(name); - nbytesRound = pad_to_int32(nbytes); - rep = calloc(1, sizeof(xXFixesGetCursorImageAndNameReply) + - npixels * sizeof(CARD32) + nbytesRound); - if (!rep) - 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; + // pixmap plus name (padded to 4 bytes) + CARD32 image[npixels + bytes_to_int32(nbytes)]; + memset(image, 0, sizeof(image)); - image = (CARD32 *) (rep + 1); CopyCursorToImage(pCursor, image); memcpy((image + npixels), name, nbytes); + + xXFixesGetCursorImageAndNameReply rep = { + .type = X_Reply, + .sequenceNumber = client->sequence, + .length = bytes_to_int32(sizeof(image)), + .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(xXFixesGetCursorImageAndNameReply), &rep); + WriteToClient(client, sizeof(image), image); return Success; } diff --git a/xfixes/disconnect.c b/xfixes/disconnect.c index 5a6c16b860..3c573e31bf 100644 --- a/xfixes/disconnect.c +++ b/xfixes/disconnect.c @@ -92,21 +92,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(xXFixesGetClientDisconnectModeReply), &rep); return Success; } diff --git a/xfixes/region.c b/xfixes/region.c index 4c22af8136..6a456c51fc 100644 --- a/xfixes/region.c +++ b/xfixes/region.c @@ -533,8 +533,6 @@ int ProcXFixesFetchRegion(ClientPtr client) { RegionPtr pRegion; - xXFixesFetchRegionReply *reply; - xRectangle *pRect; BoxPtr pExtent; BoxPtr pBox; int i, nBox; @@ -548,37 +546,36 @@ ProcXFixesFetchRegion(ClientPtr client) pBox = RegionRects(pRegion); nBox = RegionNumRects(pRegion); - reply = calloc(sizeof(xXFixesFetchRegionReply) + nBox * sizeof(xRectangle), - 1); - if (!reply) - 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; + xRectangle pRect[nBox]; - 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(xXFixesFetchRegionReply), &rep); + WriteToClient(client, sizeof(pRect), pRect); return Success; }