From 6662a7099c71e5ff9c92d1811779bba54bdbaf83 Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Wed, 24 Jul 2024 16:39:49 +0200 Subject: [PATCH] (!1601) Xext: xres: ProcXResQueryClientIds() collect reply in one buffer In order to allow simplifying the reply send path, collect the reply fragments into one buffer, instead of arbitrary number of WriteToClient() calls. This also makes it much easier for potentially new purely packet-based transports which (eg. binder) that would need their own stream parsing logic. This xres function is an exceptionally hard case, since payload is constructed step by step, and it's size only known when finished. The current way of the fragment handling still has lots of room for improvement (eg. using very small number of allocations), but leaving this for later exercise. Signed-off-by: Enrico Weigelt, metux IT consult --- Xext/xres.c | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/Xext/xres.c b/Xext/xres.c index c24b4c716..d190dc589 100644 --- a/Xext/xres.c +++ b/Xext/xres.c @@ -112,21 +112,6 @@ AddFragment(struct xorg_list *frags, int bytes) } } -/** @brief Sends all fragments in the list to the client. Does not - free anything. - - @param client The client to send the fragments to - @param frags The head of the list of fragments -*/ -static void -WriteFragmentsToClient(ClientPtr client, struct xorg_list *frags) -{ - FragmentList *it; - xorg_list_for_each_entry(it, frags, l) { - WriteToClient(client, it->bytes, (char*) it + sizeof(*it)); - } -} - /** @brief Frees a list of fragments. Does not free() root node. @param frags The head of the list of fragments @@ -564,7 +549,18 @@ ProcXResQueryClientIds (ClientPtr client) rc = ConstructClientIds(client, stuff->numSpecs, specs, &ctx); if (rc == Success) { - assert((ctx.resultBytes & 3) == 0); + char *buf = calloc(1, ctx.resultBytes); + if (!buf) { + rc = BadAlloc; + goto out; + } + char *walk = buf; + + FragmentList *it; + xorg_list_for_each_entry(it, &ctx.response, l) { + memcpy(walk, FRAGMENT_DATA(it), it->bytes); + walk += it->bytes; + } xXResQueryClientIdsReply rep = { .type = X_Reply, @@ -580,9 +576,11 @@ ProcXResQueryClientIds (ClientPtr client) } WriteToClient(client, sizeof(rep), &rep); - WriteFragmentsToClient(client, &ctx.response); + WriteToClient(client, ctx.resultBytes, buf); + free(buf); } +out: DestroyConstructClientIdCtx(&ctx); return rc; @@ -964,10 +962,24 @@ ProcXResQueryResourceBytes (ClientPtr client) SwapXResQueryResourceBytes(&ctx.response); } + char *buf = calloc(1, ctx.resultBytes); + if (!buf) { + rc = BadAlloc; + goto out; + } + + char *walk = buf; + FragmentList *it; + xorg_list_for_each_entry(it, &ctx.response, l) { + memcpy(walk, FRAGMENT_DATA(it), it->bytes); + walk += it->bytes; + } WriteToClient(client, sizeof(rep), &rep); - WriteFragmentsToClient(client, &ctx.response); + WriteToClient(client, ctx.resultBytes, buf); + free(buf); } +out: DestroyConstructResourceBytesCtx(&ctx); return rc;