[PATCH xserver] Xext/saver: Swap ScreenSaverSuspend 'suspend' field. Handle old XCB clients.

Keith Packard keithp at keithp.com
Mon Mar 12 19:27:15 UTC 2018


This field was defined as a Bool in the protocol headers and BOOL in
xcb. Bool is not a valid type for protocol fields. It is defined as
'int' by Xdefs.h, which we expect to be 32-bits on all machines.

The protocol headers and xcb have patches posted to switch to CARD32,
which is at least well defined.

This change adds the necessary byte swapping to handle other-endian
clients with this 32-bit field, and then changes the request
processing to use only the low byte of that value so that older XCB
clients will continue to work properly, at least on LSB machines.

On MSB machines, Xlib will continue to work properly, but old XCB will
not interoperate with the X server (either before or after this patch).

Signed-off-by: Keith Packard <keithp at keithp.com>
---
 Xext/saver.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Xext/saver.c b/Xext/saver.c
index 4d9e6b974..be7b403bb 100644
--- a/Xext/saver.c
+++ b/Xext/saver.c
@@ -1209,17 +1209,32 @@ static int
 ProcScreenSaverSuspend(ClientPtr client)
 {
     ScreenSaverSuspensionPtr *prev, this;
+    BOOL suspend;
 
     REQUEST(xScreenSaverSuspendReq);
     REQUEST_SIZE_MATCH(xScreenSaverSuspendReq);
 
+    /*
+     * Old versions of XCB encode suspend as 1 byte followed by three
+     * pad bytes, instead of a 4 byte value.  Be compatible on LSB
+     * machines by only using the low byte. There's not much we can do
+     * for MSB machines here as the true value will be in the high
+     * byte for old XCB and the low byte for new XCB, but always in the
+     * low byte for Xlib.
+     */
+    suspend = stuff->suspend & 0xff;
+    if (suspend != TRUE && suspend != FALSE) {
+        client->errorValue = stuff->suspend;
+        return BadValue;
+    }
+
     /* Check if this client is suspending the screensaver */
     for (prev = &suspendingClients; (this = *prev); prev = &this->next)
         if (this->pClient == client)
             break;
 
     if (this) {
-        if (stuff->suspend == TRUE)
+        if (suspend == TRUE)
             this->count++;
         else if (--this->count == 0)
             FreeResource(this->clientResource, RT_NONE);
@@ -1228,7 +1243,7 @@ ProcScreenSaverSuspend(ClientPtr client)
     }
 
     /* If we get to this point, this client isn't suspending the screensaver */
-    if (stuff->suspend == FALSE)
+    if (suspend == FALSE)
         return Success;
 
     /*
@@ -1342,6 +1357,7 @@ SProcScreenSaverSuspend(ClientPtr client)
     REQUEST(xScreenSaverSuspendReq);
 
     swaps(&stuff->length);
+    swapl(&stuff->suspend);
     REQUEST_SIZE_MATCH(xScreenSaverSuspendReq);
     return ProcScreenSaverSuspend(client);
 }
-- 
2.16.2



More information about the xorg-devel mailing list