From 2aad1a2b42b7def7812abfa2462b6bcc6382e03a Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 2 Nov 2011 14:07:19 +1000 Subject: [PATCH 1/8] include: fix mask size calculation Same bug as inputproto-2.0.1-9-gb1149ab, if the XI2LASTEVENT was a multiple of 8, the mask was one bit too short. Signed-off-by: Peter Hutterer Reviewed-by: Daniel Stone Reviewed-by: Chase Douglas --- include/inputstr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/inputstr.h b/include/inputstr.h index 0a2128732..f482a2294 100644 --- a/include/inputstr.h +++ b/include/inputstr.h @@ -72,7 +72,7 @@ extern _X_EXPORT int CountBits(const uint8_t *mask, int len); * this number here is bumped. */ #define XI2LASTEVENT 17 /* XI_RawMotion */ -#define XI2MASKSIZE ((XI2LASTEVENT + 7)/8) /* no of bits for masks */ +#define XI2MASKSIZE ((XI2LASTEVENT >> 3) + 1) /* no of bytes for masks */ /** * Scroll types for ::SetScrollValuator and the scroll type in the From bedb8fd90de8e2db33d5e3b1d859f24bf34efc9a Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 3 Nov 2011 14:25:51 +1000 Subject: [PATCH 2/8] Xi: use single return code from XIPassiveGrabDevice Some failures returned status but the actual return code was "ret". Use "ret" consistently and move status to the local block is used in. [the goto isn't necessary yet, but for a future patch] Signed-off-by: Peter Hutterer Reviewed-by: Chase Douglas --- Xi/xipassivegrab.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c index 6be27f3d1..2f13a95e8 100644 --- a/Xi/xipassivegrab.c +++ b/Xi/xipassivegrab.c @@ -80,7 +80,6 @@ ProcXIPassiveGrabDevice(ClientPtr client) DeviceIntPtr dev, mod_dev; xXIPassiveGrabDeviceReply rep; int i, ret = Success; - uint8_t status; uint32_t *modifiers; xXIGrabModifierInfo *modifiers_failed; GrabMask mask; @@ -145,32 +144,36 @@ ProcXIPassiveGrabDevice(ClientPtr client) if (stuff->cursor != None) { - status = dixLookupResourceByType(&tmp, stuff->cursor, - RT_CURSOR, client, DixUseAccess); - if (status != Success) - { - client->errorValue = stuff->cursor; - return status; - } + ret = dixLookupResourceByType(&tmp, stuff->cursor, + RT_CURSOR, client, DixUseAccess); + if (ret != Success) + { + client->errorValue = stuff->cursor; + goto out; + } } - status = dixLookupWindow((WindowPtr*)&tmp, stuff->grab_window, client, DixSetAttrAccess); - if (status != Success) - return status; + ret = dixLookupWindow((WindowPtr*)&tmp, stuff->grab_window, client, DixSetAttrAccess); + if (ret != Success) + goto out; - status = CheckGrabValues(client, ¶m); - if (status != Success) - return status; + ret = CheckGrabValues(client, ¶m); + if (ret != Success) + goto out; modifiers = (uint32_t*)&stuff[1] + stuff->mask_len; modifiers_failed = calloc(stuff->num_modifiers, sizeof(xXIGrabModifierInfo)); - if (!modifiers_failed) - return BadAlloc; + if (!modifiers_failed) { + ret = BadAlloc; + goto out; + } mod_dev = (IsFloating(dev)) ? dev : GetMaster(dev, MASTER_KEYBOARD); for (i = 0; i < stuff->num_modifiers; i++, modifiers++) { + uint8_t status = Success; + param.modifiers = *modifiers; switch(stuff->grab_type) { @@ -208,6 +211,7 @@ ProcXIPassiveGrabDevice(ClientPtr client) WriteToClient(client, rep.length * 4, (char*)modifiers_failed); free(modifiers_failed); +out: return ret; } From b371795f01c1d7fc338cfe18b8a567ed9f402846 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 3 Nov 2011 14:54:06 +1000 Subject: [PATCH 3/8] dix: rename GetWindowXI2Mask to WindowXI2MaskIsset And let it return a boolean value, that's all the callers need anyway. Signed-off-by: Peter Hutterer Reviewed-by: Chase Douglas --- dix/events.c | 13 +++++++------ include/input.h | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dix/events.c b/dix/events.c index 5e0dc5f17..35b446b2b 100644 --- a/dix/events.c +++ b/dix/events.c @@ -469,10 +469,11 @@ GetXI2MaskByte(unsigned char xi2mask[][XI2MASKSIZE], DeviceIntPtr dev, int event /** - * Return the windows complete XI2 mask for the given XI2 event type. + * @return TRUE if the mask is set for this event from this device on the + * window, or FALSE otherwise. */ -Mask -GetWindowXI2Mask(DeviceIntPtr dev, WindowPtr win, xEvent* ev) +Bool +WindowXI2MaskIsset(DeviceIntPtr dev, WindowPtr win, xEvent* ev) { OtherInputMasks *inputMasks = wOtherInputMasks(win); int filter; @@ -484,7 +485,7 @@ GetWindowXI2Mask(DeviceIntPtr dev, WindowPtr win, xEvent* ev) evtype = ((xGenericEvent*)ev)->evtype; filter = GetEventFilter(dev, ev); - return (GetXI2MaskByte(inputMasks->xi2mask, dev, evtype) & filter); + return !!(GetXI2MaskByte(inputMasks->xi2mask, dev, evtype) & filter); } Mask @@ -2075,7 +2076,7 @@ GetClientsForDelivery(DeviceIntPtr dev, WindowPtr win, { OtherInputMasks *inputMasks = wOtherInputMasks(win); /* Has any client selected for the event? */ - if (!GetWindowXI2Mask(dev, win, events)) + if (!WindowXI2MaskIsset(dev, win, events)) goto out; *clients = inputMasks->inputClients; } else { @@ -4632,7 +4633,7 @@ DeviceEnterLeaveEvent( TryClientEvents(rClient(grab), mouse, (xEvent*)event, 1, mask, filter, grab); } else { - if (!GetWindowXI2Mask(mouse, pWin, (xEvent*)event)) + if (!WindowXI2MaskIsset(mouse, pWin, (xEvent*)event)) goto out; DeliverEventsToWindow(mouse, pWin, (xEvent*)event, 1, filter, NullGrab); diff --git a/include/input.h b/include/input.h index 87bb2aa01..8e7b47a4c 100644 --- a/include/input.h +++ b/include/input.h @@ -535,7 +535,7 @@ extern _X_EXPORT void FreeInputAttributes(InputAttributes *attrs); /* misc event helpers */ extern Mask GetEventMask(DeviceIntPtr dev, xEvent* ev, InputClientsPtr clients); extern Mask GetEventFilter(DeviceIntPtr dev, xEvent *event); -extern Mask GetWindowXI2Mask(DeviceIntPtr dev, WindowPtr win, xEvent* ev); +extern Bool WindowXI2MaskIsset(DeviceIntPtr dev, WindowPtr win, xEvent* ev); void FixUpEventFromWindow(SpritePtr pSprite, xEvent *xE, WindowPtr pWin, From 4acf999c294868a44e559d212c6d88a4978256b2 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 4 Nov 2011 15:37:32 +1000 Subject: [PATCH 4/8] dix: use a single return statement in CheckPassiveGrabsOnWindow No functional change. Signed-off-by: Peter Hutterer Reviewed-by: Chase Douglas --- dix/events.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dix/events.c b/dix/events.c index 35b446b2b..4847db005 100644 --- a/dix/events.c +++ b/dix/events.c @@ -3788,14 +3788,13 @@ CheckPassiveGrabsOnWindow( } if (!activate) - { - return grab; - } + break; else if (!GetXIType(event) && !GetCoreType(event)) { ErrorF("Event type %d in CheckPassiveGrabsOnWindow is neither" " XI 1.x nor core\n", event->any.type); - return NULL; + grab = NULL; + break; } /* The only consumers of corestate are Xi 1.x and core events, which @@ -3861,9 +3860,10 @@ CheckPassiveGrabsOnWindow( } free(xE); - return grab; + break; } - return NULL; + + return grab; #undef CORE_MATCH #undef XI_MATCH #undef XI2_MATCH From ee9346bb31efce4036df1b8dd8f1a5dc43ae955a Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 3 Nov 2011 15:45:34 +1000 Subject: [PATCH 5/8] Xi: add helper functions to alloc/free InputClientPtrs Currently not needed since the InputClientRec is a self-contained struct. As part of the touch rework that won't be the case in the future and a function to allocate/free memory appropriately is required. Signed-off-by: Peter Hutterer Reviewed-by: Chase Douglas --- Xi/exevents.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/Xi/exevents.c b/Xi/exevents.c index 53db03629..f53031013 100644 --- a/Xi/exevents.c +++ b/Xi/exevents.c @@ -1628,6 +1628,19 @@ SelectForWindow(DeviceIntPtr dev, WindowPtr pWin, ClientPtr client, return Success; } +static void +FreeInputClient(InputClientsPtr *other) +{ + free(*other); + *other = NULL; +} + +static InputClientsPtr +AllocInputClient(void) +{ + return calloc(1, sizeof(InputClients)); +} + int AddExtensionClient(WindowPtr pWin, ClientPtr client, Mask mask, int mskidx) { @@ -1635,7 +1648,7 @@ AddExtensionClient(WindowPtr pWin, ClientPtr client, Mask mask, int mskidx) if (!pWin->optional && !MakeWindowOptional(pWin)) return BadAlloc; - others = calloc(1, sizeof(InputClients)); + others = AllocInputClient(); if (!others) return BadAlloc; if (!pWin->optional->inputMasks && !MakeInputMasks(pWin)) @@ -1649,7 +1662,7 @@ AddExtensionClient(WindowPtr pWin, ClientPtr client, Mask mask, int mskidx) return Success; bail: - free(others); + FreeInputClient(&others); return BadAlloc; } @@ -1721,14 +1734,14 @@ InputClientGone(WindowPtr pWin, XID id) if (other->resource == id) { if (prev) { prev->next = other->next; - free(other); + FreeInputClient(&other); } else if (!(other->next)) { if (ShouldFreeInputMasks(pWin, TRUE)) { wOtherInputMasks(pWin)->inputClients = other->next; free(wOtherInputMasks(pWin)); pWin->optional->inputMasks = (OtherInputMasks *) NULL; CheckWindowOptionalNeed(pWin); - free(other); + FreeInputClient(&other); } else { other->resource = FakeClientID(0); if (!AddResource(other->resource, RT_INPUTCLIENT, @@ -1737,7 +1750,7 @@ InputClientGone(WindowPtr pWin, XID id) } } else { wOtherInputMasks(pWin)->inputClients = other->next; - free(other); + FreeInputClient(&other); } RecalculateDeviceDeliverableEvents(pWin); return Success; From 61ef4daf6450da573b9de72ba7b200566821b916 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 4 Nov 2011 15:49:23 +1000 Subject: [PATCH 6/8] Xi: add FreeInputMask function Does what it says on the box, complements MakeInputMask. Signed-off-by: Peter Hutterer Reviewed-by: Chase Douglas --- Xi/exevents.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Xi/exevents.c b/Xi/exevents.c index f53031013..20495e74d 100644 --- a/Xi/exevents.c +++ b/Xi/exevents.c @@ -1678,6 +1678,13 @@ MakeInputMasks(WindowPtr pWin) return TRUE; } +static void +FreeInputMask(OtherInputMasks **imask) +{ + free(*imask); + *imask = NULL; +} + void RecalculateDeviceDeliverableEvents(WindowPtr pWin) { @@ -1737,8 +1744,9 @@ InputClientGone(WindowPtr pWin, XID id) FreeInputClient(&other); } else if (!(other->next)) { if (ShouldFreeInputMasks(pWin, TRUE)) { - wOtherInputMasks(pWin)->inputClients = other->next; - free(wOtherInputMasks(pWin)); + OtherInputMasks *mask = wOtherInputMasks(pWin); + mask->inputClients = other->next; + FreeInputMask(&mask); pWin->optional->inputMasks = (OtherInputMasks *) NULL; CheckWindowOptionalNeed(pWin); FreeInputClient(&other); From 9c38422fc4cf853c3104fada2a3851c79df2a66f Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 9 Nov 2011 14:37:26 +1000 Subject: [PATCH 7/8] include: add BUG_WARN macro for internal bug cases. There are plenty of cases that can only be triggered by a real bug in the server and doing the ErrorF dance manually everywhere is a tad painful and the error message is usually used only to find the spot in the file anyway. Plus, reading BUG_WARN somewhere is a good indicator to the casual reader that this isn't intended behaviour. Note that this is intentionally different to the BUG_ON behaviour on the kernel, we do not FatalError the server. It's just a warning + stacktrace. If the bug is really fatal, call FatalError. Signed-off-by: Peter Hutterer Reviewed-by: Chase Douglas Reviewed-by: Alan Coopersmith --- include/misc.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/misc.h b/include/misc.h index 2e8dd1b89..ef86fa539 100644 --- a/include/misc.h +++ b/include/misc.h @@ -359,4 +359,11 @@ typedef struct _CharInfo *CharInfoPtr; /* also in fonts/include/font.h */ extern _X_EXPORT unsigned long globalSerialNumber; extern _X_EXPORT unsigned long serverGeneration; +#define BUG_WARN(cond) \ + do { if (cond) { \ + ErrorF("BUG: triggered 'if (" #cond ")'\nBUG: %s:%d in %s()\n", \ + __FILE__, __LINE__, __func__); \ + xorg_backtrace(); \ + } } while(0) + #endif /* MISC_H */ From dfcec1d3f9d7bac5cde9eb034341428ee0ad3728 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 11 Nov 2011 08:55:14 +1000 Subject: [PATCH 8/8] test: remove unneeded printf statements from misc.c Leftover from debugging, is not really needeed in a test. Signed-off-by: Peter Hutterer Reviewed-by: Alan Coopersmith --- test/misc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/misc.c b/test/misc.c index d98449ba0..1f6cb9a94 100644 --- a/test/misc.c +++ b/test/misc.c @@ -76,7 +76,6 @@ static void dix_update_desktop_dimensions(void) #define assert_dimensions(_x, _y, _w, _h) \ update_desktop_dimensions(); \ - printf("%d %d %d %d\n", screenInfo.x, screenInfo.y, screenInfo.width, screenInfo.height); \ assert(screenInfo.x == _x); \ assert(screenInfo.y == _y); \ assert(screenInfo.width == _w); \ @@ -88,8 +87,6 @@ static void dix_update_desktop_dimensions(void) screenInfo.screens[idx]->width = _w; \ screenInfo.screens[idx]->height = _h; \ - printf("Testing\n"); - /* single screen */ screenInfo.numScreens = 1; set_screen(0, x, y, w, h);