[PATCH 2/3] Make WriteEventsToClient/WriteToClient no-op on fake or dead clients.

Jamey Sharp jamey at minilop.net
Sat May 15 13:52:14 PDT 2010


This matches the test in TryClientEvents, and is a superset of tests
done by the callers of these functions. The consequence of forgetting
these tests is a server crash, so they're always desirable. In my
opinion, it's better to not require the callers to remember to do these
checks.

For callers that don't do very much work before calling WriteToClient or
WriteEventsToClient, I've removed the redundant checks.

hw/xquartz/xpr/appledri.c has an interesting case: While its check for
"client == NULL" appears redundant with the test in WriteEventsToClient,
it dereferences client to get the sequence number.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=27497
Signed-off-by: Jamey Sharp <jamey at minilop.net>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
---
Chris, you can add my reviewed-by to "dri2: Only send the swap event if
the client hasn't gone fishing", but I think this is a better fix, in
that hopefully nobody will ever have this problem again. :-) What do you
think?

 Xext/saver.c                          |    2 --
 Xext/security.c                       |   14 +++++---------
 Xext/shape.c                          |    2 --
 Xext/sync.c                           |    9 +++------
 Xext/xcalibrate.c                     |    3 +--
 damageext/damageext.c                 |    6 ++----
 dix/events.c                          |    3 +++
 hw/xfree86/dixmods/extmod/xf86vmode.c |    2 --
 hw/xfree86/dri2/dri2ext.c             |    3 ---
 hw/xquartz/applewm.c                  |    5 +----
 hw/xquartz/xpr/appledri.c             |    2 +-
 hw/xwin/winwindowswm.c                |    3 +--
 os/io.c                               |    8 +++++---
 randr/rrproperty.c                    |    7 +------
 xfixes/cursor.c                       |    3 +--
 xfixes/select.c                       |    3 +--
 16 files changed, 25 insertions(+), 50 deletions(-)

diff --git a/Xext/saver.c b/Xext/saver.c
index 4b43a30..42fc632 100644
--- a/Xext/saver.c
+++ b/Xext/saver.c
@@ -500,8 +500,6 @@ SendScreenSaverNotify (ScreenPtr pScreen, int state, Bool forced)
     for (pEv = pPriv->events; pEv; pEv = pEv->next)
     {
 	client = pEv->client;
-	if (client->clientGone)
-	    continue;
 	if (!(pEv->mask & mask))
 	    continue;
 	ev.type = ScreenSaverNotify + ScreenSaverEventBase;
diff --git a/Xext/security.c b/Xext/security.c
index 7995ff2..16aac05 100644
--- a/Xext/security.c
+++ b/Xext/security.c
@@ -199,15 +199,11 @@ SecurityDeleteAuthorization(
     {
 	/* send revocation event event */
 	ClientPtr client = rClient(pEventClient);
-
-	if (!client->clientGone)
-	{
-	    xSecurityAuthorizationRevokedEvent are;
-	    are.type = SecurityEventBase + XSecurityAuthorizationRevoked;
-	    are.sequenceNumber = client->sequence;
-	    are.authId = pAuth->id;
-	    WriteEventsToClient(client, 1, (xEvent *)&are);
-	}
+	xSecurityAuthorizationRevokedEvent are;
+	are.type = SecurityEventBase + XSecurityAuthorizationRevoked;
+	are.sequenceNumber = client->sequence;
+	are.authId = pAuth->id;
+	WriteEventsToClient(client, 1, (xEvent *)&are);
 	FreeResource(pEventClient->resource, RT_NONE);
     }
 
diff --git a/Xext/shape.c b/Xext/shape.c
index cd75658..10437f4 100644
--- a/Xext/shape.c
+++ b/Xext/shape.c
@@ -941,8 +941,6 @@ SendShapeNotify (WindowPtr pWin, int which)
     }
     for (pShapeEvent = *pHead; pShapeEvent; pShapeEvent = pShapeEvent->next) {
 	client = pShapeEvent->client;
-	if (client == serverClient || client->clientGone)
-	    continue;
 	se.type = ShapeNotify + ShapeEventBase;
 	se.kind = which;
 	se.window = pWin->drawable.id;
diff --git a/Xext/sync.c b/Xext/sync.c
index d46087a..3729f1b 100644
--- a/Xext/sync.c
+++ b/Xext/sync.c
@@ -390,17 +390,14 @@ SyncSendAlarmNotifyEvents(SyncAlarm *pAlarm)
     ane.state = pAlarm->state;
 
     /* send to owner */
-    if (pAlarm->events && !pAlarm->client->clientGone)
+    if (pAlarm->events)
 	WriteEventsToClient(pAlarm->client, 1, (xEvent *) &ane);
 
     /* send to other interested clients */
     for (pcl = pAlarm->pEventClients; pcl; pcl = pcl->next)
     {
-	if (!pcl->client->clientGone)
-	{
-	    ane.sequenceNumber = pcl->client->sequence;
-	    WriteEventsToClient(pcl->client, 1, (xEvent *) &ane);
-	}
+	ane.sequenceNumber = pcl->client->sequence;
+	WriteEventsToClient(pcl->client, 1, (xEvent *) &ane);
     }
 }
 
diff --git a/Xext/xcalibrate.c b/Xext/xcalibrate.c
index 364b92a..8659384 100644
--- a/Xext/xcalibrate.c
+++ b/Xext/xcalibrate.c
@@ -59,8 +59,7 @@ xcalibrate_event_hook (int x, int y, int pressure, void *closure)
   ev.y = y;
   ev.pressure = pressure;
 
-  if (!pClient->clientGone)
-    WriteEventsToClient (pClient, 1, (xEvent *) &ev);
+  WriteEventsToClient (pClient, 1, (xEvent *) &ev);
 }
 
 static int
diff --git a/damageext/damageext.c b/damageext/damageext.c
index af4fef6..c80554e 100644
--- a/damageext/damageext.c
+++ b/damageext/damageext.c
@@ -69,8 +69,7 @@ DamageExtNotify (DamageExtPtr pDamageExt, BoxPtr pBoxes, int nBoxes)
 	    ev.area.y = pBoxes[i].y1;
 	    ev.area.width = pBoxes[i].x2 - pBoxes[i].x1;
 	    ev.area.height = pBoxes[i].y2 - pBoxes[i].y1;
-	    if (!pClient->clientGone)
-		WriteEventsToClient (pClient, 1, (xEvent *) &ev);
+	    WriteEventsToClient (pClient, 1, (xEvent *) &ev);
 	}
     }
     else
@@ -79,8 +78,7 @@ DamageExtNotify (DamageExtPtr pDamageExt, BoxPtr pBoxes, int nBoxes)
 	ev.area.y = 0;
 	ev.area.width = pDrawable->width;
 	ev.area.height = pDrawable->height;
-	if (!pClient->clientGone)
-	    WriteEventsToClient (pClient, 1, (xEvent *) &ev);
+	WriteEventsToClient (pClient, 1, (xEvent *) &ev);
     }
     /* Composite extension marks clients with manual Subwindows as critical */
     if (pDamageClient->critical > 0)
diff --git a/dix/events.c b/dix/events.c
index 3ed344d..a00ecd9 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -5669,6 +5669,9 @@ WriteEventsToClient(ClientPtr pClient, int count, xEvent *events)
     int       i,
               eventlength = sizeof(xEvent);
 
+    if (!pClient || pClient == serverClient || pClient->clientGone)
+	return;
+
     /* Let XKB rewrite the state, as it depends on client preferences. */
     XkbFilterEvents(pClient, count, events);
 
diff --git a/hw/xfree86/dixmods/extmod/xf86vmode.c b/hw/xfree86/dixmods/extmod/xf86vmode.c
index 665f743..0b7f75e 100644
--- a/hw/xfree86/dixmods/extmod/xf86vmode.c
+++ b/hw/xfree86/dixmods/extmod/xf86vmode.c
@@ -344,8 +344,6 @@ SendXF86VidModeNotify(ScreenPtr pScreen, int state, Bool forced)
     for (pEv = pPriv->events; pEv; pEv = pEv->next)
     {
 	client = pEv->client;
-	if (client->clientGone)
-	    continue;
 	if (!(pEv->mask & mask))
 	    continue;
 	ev.type = XF86VidModeNotify + XF86VidModeEventBase;
diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
index 44a47cc..444751d 100644
--- a/hw/xfree86/dri2/dri2ext.c
+++ b/hw/xfree86/dri2/dri2ext.c
@@ -160,9 +160,6 @@ DRI2InvalidateBuffersEvent(DrawablePtr pDraw, void *priv)
     xDRI2InvalidateBuffers event;
     ClientPtr client = priv;
 
-    if (client->clientGone)
-	return;
-
     event.type = DRI2EventBase + DRI2_InvalidateBuffers;
     event.sequenceNumber = client->sequence;
     event.drawable = pDraw->id;
diff --git a/hw/xquartz/applewm.c b/hw/xquartz/applewm.c
index 53d167e..8c248ed 100644
--- a/hw/xquartz/applewm.c
+++ b/hw/xquartz/applewm.c
@@ -350,11 +350,8 @@ AppleWMSendEvent (int type, unsigned int mask, int which, int arg) {
         return;
     for (pEvent = *pHead; pEvent; pEvent = pEvent->next) {
         client = pEvent->client;
-        if ((pEvent->mask & mask) == 0
-            || client == serverClient || client->clientGone)
-        {
+        if ((pEvent->mask & mask) == 0)
             continue;
-        }
         se.type = type + WMEventBase;
         se.kind = which;
         se.arg = arg;
diff --git a/hw/xquartz/xpr/appledri.c b/hw/xquartz/xpr/appledri.c
index 71cfb59..0fbe850 100644
--- a/hw/xquartz/xpr/appledri.c
+++ b/hw/xquartz/xpr/appledri.c
@@ -199,7 +199,7 @@ static void surface_notify(
         return;
 
     client = clients[client_index];
-    if (client == NULL || client == serverClient || client->clientGone)
+    if (client == NULL)
         return;
 
     se.type = DRIEventBase + AppleDRISurfaceNotify;
diff --git a/hw/xwin/winwindowswm.c b/hw/xwin/winwindowswm.c
index d09e983..b534bd5 100755
--- a/hw/xwin/winwindowswm.c
+++ b/hw/xwin/winwindowswm.c
@@ -304,8 +304,7 @@ winWindowsWMSendEvent (int type, unsigned int mask, int which, int arg,
 #if CYGMULTIWINDOW_DEBUG
       ErrorF ("winWindowsWMSendEvent - x%08x\n", (int) client);
 #endif
-      if ((pEvent->mask & mask) == 0
-	  || client == serverClient || client->clientGone)
+      if ((pEvent->mask & mask) == 0)
 	{
 	  continue;
 	}
diff --git a/os/io.c b/os/io.c
index 8335102..02e1ca3 100644
--- a/os/io.c
+++ b/os/io.c
@@ -698,15 +698,17 @@ SetCriticalOutputPending(void)
 int
 WriteToClient (ClientPtr who, int count, const void *__buf)
 {
-    OsCommPtr oc = (OsCommPtr)who->osPrivate;
-    ConnectionOutputPtr oco = oc->output;
+    OsCommPtr oc;
+    ConnectionOutputPtr oco;
     int padBytes;
     const char *buf = __buf;
 #ifdef DEBUG_COMMUNICATION
     Bool multicount = FALSE;
 #endif
-    if (!count)
+    if (!count || !who || who == serverClient || who->clientGone)
 	return(0);
+    oc = who->osPrivate;
+    oco = oc->output;
 #ifdef DEBUG_COMMUNICATION
     {
 	char info[128];
diff --git a/randr/rrproperty.c b/randr/rrproperty.c
index ff0bca0..3aab37a 100644
--- a/randr/rrproperty.c
+++ b/randr/rrproperty.c
@@ -29,7 +29,6 @@ DeliverPropertyEvent(WindowPtr pWin, void *value)
 {
     xRROutputPropertyNotifyEvent *event = value;
     RREventPtr *pHead, pRREvent;
-    ClientPtr client;
 
     dixLookupResourceByType((pointer *)&pHead, pWin->drawable.id,
 			    RREventType, serverClient, DixReadAccess);
@@ -38,14 +37,10 @@ DeliverPropertyEvent(WindowPtr pWin, void *value)
 
     for (pRREvent = *pHead; pRREvent; pRREvent = pRREvent->next)
     {
-	client = pRREvent->client;
-	if (client == serverClient || client->clientGone)
-	    continue;
-
 	if (!(pRREvent->mask & RROutputPropertyNotifyMask))
 	    continue;
 
-	event->sequenceNumber = client->sequence;
+	event->sequenceNumber = pRREvent->client->sequence;
 	event->window = pRREvent->window->drawable.id;
 	WriteEventsToClient(pRREvent->client, 1, (xEvent *)event);
     }
diff --git a/xfixes/cursor.c b/xfixes/cursor.c
index d5f8b29..e963069 100644
--- a/xfixes/cursor.c
+++ b/xfixes/cursor.c
@@ -164,8 +164,7 @@ CursorDisplayCursor (DeviceIntPtr pDev,
 	CursorCurrent[pDev->id] = pCursor;
 	for (e = cursorEvents; e; e = e->next)
 	{
-	    if ((e->eventMask & XFixesDisplayCursorNotifyMask) &&
-		!e->pClient->clientGone)
+	    if ((e->eventMask & XFixesDisplayCursorNotifyMask))
 	    {
 		xXFixesCursorNotifyEvent	ev;
 		ev.type = XFixesEventBase + XFixesCursorNotify;
diff --git a/xfixes/select.c b/xfixes/select.c
index 6d86f63..ffd1c69 100644
--- a/xfixes/select.c
+++ b/xfixes/select.c
@@ -78,8 +78,7 @@ XFixesSelectionCallback (CallbackListPtr *callbacks, pointer data, pointer args)
     for (e = selectionEvents; e; e = e->next)
     {
 	if (e->selection == selection->selection && 
-	    (e->eventMask & eventMask) &&
-	    !e->pClient->clientGone)
+	    (e->eventMask & eventMask))
 	{
 	    xXFixesSelectionNotifyEvent	ev;
 
-- 
1.7.0



More information about the xorg-devel mailing list