From 81daba8ce906bfbbe44cd71d0ff269ad34e2f6b5 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 31 Jan 2011 13:53:08 +1000 Subject: [PATCH 01/10] Xi: constify XIChangeDeviceProperty() We don't modify "value", make it official. Signed-off-by: Peter Hutterer Reviewed-by: Chase Douglas Signed-off-by: Ander Conselvan de Oliveira --- Xi/xiproperty.c | 2 +- include/exevents.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Xi/xiproperty.c b/Xi/xiproperty.c index b9f53f7dc..17835e2cd 100644 --- a/Xi/xiproperty.c +++ b/Xi/xiproperty.c @@ -701,7 +701,7 @@ XIDeleteDeviceProperty (DeviceIntPtr device, Atom property, Bool fromClient) int XIChangeDeviceProperty (DeviceIntPtr dev, Atom property, Atom type, int format, int mode, unsigned long len, - pointer value, Bool sendevent) + const pointer value, Bool sendevent) { XIPropertyPtr prop; int size_in_bytes; diff --git a/include/exevents.h b/include/exevents.h index dc594304f..2b226986b 100644 --- a/include/exevents.h +++ b/include/exevents.h @@ -69,7 +69,7 @@ extern _X_EXPORT int XIChangeDeviceProperty( int /* format*/, int /* mode*/, unsigned long /* len*/, - pointer /* value*/, + const pointer /* value*/, Bool /* sendevent*/ ); From 0ef5973860e17c5edc996c923610f7ad88b4dfbe Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Tue, 8 Feb 2011 11:10:08 +0200 Subject: [PATCH 02/10] ProcRRQueryVersion: fix use of uninitialised bytes valgrind error. ==9999== Syscall param writev(vector[...]) points to uninitialised byte(s) ==9999== at 0x4AB5154: writev (writev.c:51) ==9999== by 0x7C7C3: _XSERVTransWritev (Xtrans.c:912) ==9999== by 0x61C8B: FlushClient (io.c:924) ==9999== by 0x62743: FlushAllOutput (io.c:668) ==9999== by 0x4AA5B: Dispatch (dispatch.c:453) ==9999== by 0x205BF: main (main.c:291) ==9999== Address 0x55711b9 is 1 bytes inside a block of size 4,096 alloc'd ==9999== at 0x48334A4: calloc (vg_replace_malloc.c:467) ==9999== by 0x62567: WriteToClient (io.c:1065) ==9999== by 0x452EB: ProcEstablishConnection (dispatch.c:3685) ==9999== by 0x4AB53: Dispatch (dispatch.c:432) ==9999== by 0x205BF: main (main.c:291) ==9999== Uninitialised value was created by a stack allocation ==9999== at 0x160E78: ProcRRQueryVersion (rrdispatch.c:37) Signed-off-by: Peter Hutterer Reviewed-by: Oliver McFadden Signed-off-by: Ander Conselvan de Oliveira --- randr/rrdispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/randr/rrdispatch.c b/randr/rrdispatch.c index aed746bac..b0b451c2a 100644 --- a/randr/rrdispatch.c +++ b/randr/rrdispatch.c @@ -35,7 +35,7 @@ RRClientKnowsRates (ClientPtr pClient) static int ProcRRQueryVersion (ClientPtr client) { - xRRQueryVersionReply rep; + xRRQueryVersionReply rep = {0}; register int n; REQUEST(xRRQueryVersionReq); rrClientPriv(client); From 87fbef9157a6f1e1318382e368d27942d7ad72ab Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Tue, 8 Feb 2011 11:10:09 +0200 Subject: [PATCH 03/10] ProcRRCreateMode: fix use of uninitialised bytes valgrind error. ==543== Syscall param writev(vector[...]) points to uninitialised byte(s) ==543== at 0x4AB7154: writev (writev.c:51) ==543== by 0x8935B: _XSERVTransWritev (Xtrans.c:912) ==543== by 0x6C55F: FlushClient (io.c:924) ==543== by 0x6D013: FlushAllOutput (io.c:668) ==543== by 0x27A83: Dispatch (dispatch.c:453) ==543== by 0x205B7: main (main.c:291) ==543== Address 0x556dc8c is 12 bytes inside a block of size 4,096 alloc'd ==543== at 0x48334A4: calloc (vg_replace_malloc.c:467) ==543== by 0x6CE37: WriteToClient (io.c:1065) ==543== by 0x223A7: ProcEstablishConnection (dispatch.c:3685) ==543== by 0x27B7B: Dispatch (dispatch.c:432) ==543== by 0x205B7: main (main.c:291) ==543== Uninitialised value was created by a stack allocation ==543== at 0xA3350: ProcRRCreateMode (rrmode.c:289) Signed-off-by: Peter Hutterer Reviewed-by: Oliver McFadden Signed-off-by: Ander Conselvan de Oliveira --- randr/rrmode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/randr/rrmode.c b/randr/rrmode.c index 5ffa4006f..d7560dcb2 100644 --- a/randr/rrmode.c +++ b/randr/rrmode.c @@ -288,7 +288,7 @@ int ProcRRCreateMode (ClientPtr client) { REQUEST(xRRCreateModeReq); - xRRCreateModeReply rep; + xRRCreateModeReply rep = {0}; WindowPtr pWin; ScreenPtr pScreen; rrScrPrivPtr pScrPriv; From 8a34d7a8532c7ca013e67307f3baf200167abb92 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Tue, 8 Feb 2011 11:10:11 +0200 Subject: [PATCH 04/10] XkbSendNames: fix use of uninitialised bytes valgrind error. ==537== Syscall param writev(vector[...]) points to uninitialised byte(s) ==537== at 0x4AB7154: writev (writev.c:51) ==537== by 0x8935B: _XSERVTransWritev (Xtrans.c:912) ==537== by 0x6C55F: FlushClient (io.c:924) ==537== by 0x6CCF3: WriteToClient (io.c:846) ==537== by 0xD51D3: XkbSendNames (xkb.c:3765) ==537== by 0xD8183: ProcXkbGetKbdByName (xkb.c:5825) ==537== by 0x27B7B: Dispatch (dispatch.c:432) ==537== by 0x205B7: main (main.c:291) ==537== Address 0x55899f2 is 154 bytes inside a block of size 1,896 alloc'd ==537== at 0x4834C48: malloc (vg_replace_malloc.c:236) ==537== by 0xD47AF: XkbSendNames (xkb.c:3642) ==537== by 0xD8183: ProcXkbGetKbdByName (xkb.c:5825) ==537== by 0x27B7B: Dispatch (dispatch.c:432) ==537== by 0x205B7: main (main.c:291) ==537== Uninitialised value was created by a heap allocation ==537== at 0x4834C48: malloc (vg_replace_malloc.c:236) ==537== by 0xD47AF: XkbSendNames (xkb.c:3642) ==537== by 0xD8183: ProcXkbGetKbdByName (xkb.c:5825) ==537== by 0x27B7B: Dispatch (dispatch.c:432) ==537== by 0x205B7: main (main.c:291) Signed-off-by: Peter Hutterer Reviewed-by: Oliver McFadden Signed-off-by: Ander Conselvan de Oliveira --- xkb/xkb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xkb/xkb.c b/xkb/xkb.c index 6fd66c5e5..a2cbbf315 100644 --- a/xkb/xkb.c +++ b/xkb/xkb.c @@ -3644,7 +3644,7 @@ register int n; swapl(&rep->indicators,n); } - start = desc = malloc(length); + start = desc = calloc(1, length); if ( !start ) return BadAlloc; if (xkb->names) { From 85f9017393c9bb19553e9afcf554673a44a09993 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Tue, 8 Feb 2011 11:10:10 +0200 Subject: [PATCH 05/10] ProcXkbGetXkbByName: fix use of uninitialised bytes valgrind error. ==9999== Syscall param writev(vector[...]) points to uninitialised byte(s) ==9999== at 0x4AB5154: writev (writev.c:51) ==9999== by 0x7C7C3: _XSERVTransWritev (Xtrans.c:912) ==9999== by 0x61C8B: FlushClient (io.c:924) ==9999== by 0x62423: WriteToClient (io.c:846) ==9999== by 0xCE39B: XkbSendMap (xkb.c:1408) ==9999== by 0xD247B: ProcXkbGetKbdByName (xkb.c:5814) ==9999== by 0x4AB53: Dispatch (dispatch.c:432) ==9999== by 0x205BF: main (main.c:291) ==9999== Address 0x557eb68 is 40 bytes inside a block of size 4,096 alloc'd ==9999== at 0x48334A4: calloc (vg_replace_malloc.c:467) ==9999== by 0x62567: WriteToClient (io.c:1065) ==9999== by 0x452EB: ProcEstablishConnection (dispatch.c:3685) ==9999== by 0x4AB53: Dispatch (dispatch.c:432) ==9999== by 0x205BF: main (main.c:291) ==9999== Uninitialised value was created by a stack allocation ==9999== at 0xD1910: ProcXkbGetKbdByName (xkb.c:5559) Signed-off-by: Peter Hutterer Reviewed-by: Oliver McFadden Signed-off-by: Ander Conselvan de Oliveira --- xkb/xkb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/xkb/xkb.c b/xkb/xkb.c index a2cbbf315..a57139f35 100644 --- a/xkb/xkb.c +++ b/xkb/xkb.c @@ -5569,13 +5569,13 @@ ProcXkbGetKbdByName(ClientPtr client) { DeviceIntPtr dev; DeviceIntPtr tmpd; - xkbGetKbdByNameReply rep; - xkbGetMapReply mrep; - xkbGetCompatMapReply crep; - xkbGetIndicatorMapReply irep; - xkbGetNamesReply nrep; - xkbGetGeometryReply grep; - XkbComponentNamesRec names; + xkbGetKbdByNameReply rep = {0}; + xkbGetMapReply mrep = {0}; + xkbGetCompatMapReply crep = {0}; + xkbGetIndicatorMapReply irep = {0}; + xkbGetNamesReply nrep = {0}; + xkbGetGeometryReply grep = {0}; + XkbComponentNamesRec names = {0}; XkbDescPtr xkb, new; unsigned char * str; char mapFile[PATH_MAX]; From 787ba25a8a3af52b38448a1a6f8c9704ea8b7905 Mon Sep 17 00:00:00 2001 From: Carlos Garnacho Date: Mon, 7 Feb 2011 18:21:31 +0100 Subject: [PATCH 06/10] Xi: make XIQueryPointer return the current modifiers/group as documented. The previous XKB info was being returned instead of the current one, producing inconsistent results between the latest events and the modifiers/group returned by this call. Signed-off-by: Carlos Garnacho Reviewed-by: Daniel Stone Reviewed-by: Peter Hutterer ` Signed-off-by: Peter Hutterer --- Xi/xiquerypointer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Xi/xiquerypointer.c b/Xi/xiquerypointer.c index b521c48ef..8df958ea2 100644 --- a/Xi/xiquerypointer.c +++ b/Xi/xiquerypointer.c @@ -129,7 +129,7 @@ ProcXIQueryPointer(ClientPtr client) if (kbd) { - state = &kbd->key->xkbInfo->prev_state; + state = &kbd->key->xkbInfo->state; rep.mods.base_mods = state->base_mods; rep.mods.latched_mods = state->latched_mods; rep.mods.locked_mods = state->locked_mods; From 47d1d2fed656c3a3b2600491078da90962c46934 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 10 Feb 2011 15:11:34 +1000 Subject: [PATCH 07/10] xkb: split out keymap compilation. Refactoring for simpler double-use in the next patch. No functional changes. Signed-off-by: Peter Hutterer Reviewed-by: Dan Nicholson --- xkb/ddxLoad.c | 57 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c index 51b577725..b968c4360 100644 --- a/xkb/ddxLoad.c +++ b/xkb/ddxLoad.c @@ -425,35 +425,58 @@ XkbRF_RulesPtr rules; return complete; } -XkbDescPtr -XkbCompileKeymap(DeviceIntPtr dev, XkbRMLVOSet *rmlvo) +static Bool +XkbRMLVOtoKcCGST(DeviceIntPtr dev, XkbRMLVOSet *rmlvo, XkbComponentNamesPtr kccgst) { - XkbComponentNamesRec kccgst; XkbRF_VarDefsRec mlvo; - XkbDescPtr xkb; - char name[PATH_MAX]; - - if (!dev || !rmlvo) { - LogMessage(X_ERROR, "XKB: No device or RMLVO specified\n"); - return NULL; - } mlvo.model = rmlvo->model; mlvo.layout = rmlvo->layout; mlvo.variant = rmlvo->variant; mlvo.options = rmlvo->options; - /* XDNFR already logs for us. */ - if (!XkbDDXNamesFromRules(dev, rmlvo->rules, &mlvo, &kccgst)) + return XkbDDXNamesFromRules(dev, rmlvo->rules, &mlvo, kccgst); +} + +/** + * Compile the given RMLVO keymap and return it. Returns the XkbDescPtr on + * success or NULL on failure. If the components compiled are not a superset + * or equal to need, the compiliation is treated as failure. + */ +static XkbDescPtr +XkbCompileKeymapForDevice(DeviceIntPtr dev, XkbRMLVOSet *rmlvo, int need) +{ + XkbDescPtr xkb; + unsigned int provided; + XkbComponentNamesRec kccgst; + char name[PATH_MAX]; + + if (!XkbRMLVOtoKcCGST(dev, rmlvo, &kccgst)) return NULL; - /* XDLKBN too, but it might return 0 as well as allocating. */ - if (!XkbDDXLoadKeymapByNames(dev, &kccgst, XkmAllIndicesMask, 0, &xkb, name, - PATH_MAX)) { - if (xkb) + provided = XkbDDXLoadKeymapByNames(dev, &kccgst, XkmAllIndicesMask, need, + &xkb, name, PATH_MAX); + if ((need & provided) != need) { + if (xkb) { XkbFreeKeyboard(xkb, 0, TRUE); - return NULL; + xkb = NULL; + } } return xkb; } + +XkbDescPtr +XkbCompileKeymap(DeviceIntPtr dev, XkbRMLVOSet *rmlvo) +{ + XkbDescPtr xkb; + + if (!dev || !rmlvo) { + LogMessage(X_ERROR, "XKB: No device or RMLVO specified\n"); + return NULL; + } + + xkb = XkbCompileKeymapForDevice(dev, rmlvo, 0); + + return xkb; +} From d3499556d8d83396fa2585bd00371a81e086be36 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 10 Feb 2011 15:12:14 +1000 Subject: [PATCH 08/10] xkb: if the keymap failed to compile, load the default keymap instead. We really need symbols, compat, keynames, vmods and types for a sensible keymap. Try this in your xorg.conf.d snippets for all keyboards: Option "XkbLayout" "us" Option "XkbVariant" "nodeadkeys" us(nodeadkeys) doesn't exist so xkbcomp provides everything but the symbols map. We say we want everything but don't _need_ anything, the server happily gives us a keymap with every key mapped to NoSymbol. This in turn isn't what we want after all. So instead, require symbols, compat, keynames, vmods and types from the keymap and if that fails, load the default keymap instead. If that fails too, all bets are off. Signed-off-by: Peter Hutterer Reviewed-by: Dan Nicholson --- xkb/ddxLoad.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c index b968c4360..ac587fc9d 100644 --- a/xkb/ddxLoad.c +++ b/xkb/ddxLoad.c @@ -470,13 +470,34 @@ XkbDescPtr XkbCompileKeymap(DeviceIntPtr dev, XkbRMLVOSet *rmlvo) { XkbDescPtr xkb; + unsigned int need; if (!dev || !rmlvo) { LogMessage(X_ERROR, "XKB: No device or RMLVO specified\n"); return NULL; } - xkb = XkbCompileKeymapForDevice(dev, rmlvo, 0); + /* These are the components we really really need */ + need = XkmSymbolsMask | XkmCompatMapMask | XkmTypesMask | + XkmKeyNamesMask | XkmVirtualModsMask; + + + xkb = XkbCompileKeymapForDevice(dev, rmlvo, need); + + if (!xkb) { + XkbRMLVOSet dflts; + + /* we didn't get what we really needed. And that will likely leave + * us with a keyboard that doesn't work. Use the defaults instead */ + LogMessage(X_ERROR, "XKB: Failed to load keymap. Loading default " + "keymap instead.\n"); + + XkbGetRulesDflts(&dflts); + + xkb = XkbCompileKeymapForDevice(dev, &dflts, 0); + + XkbFreeRMLVOSet(&dflts, FALSE); + } return xkb; } From c9f7b303a36ca501c6ecf1196c266ee8e8f49d2d Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 11 Feb 2011 13:50:10 +1000 Subject: [PATCH 09/10] xfixes: calloc, not malloc the cursorScreenRec Debugging NULL pointers is significantly easier than random memory. Plus, if new fields (such as pointer barriers) are added they may just be properly initialised. Signed-off-by: Peter Hutterer Reviewed-by: Keith Packard --- xfixes/cursor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xfixes/cursor.c b/xfixes/cursor.c index 54e5d755e..fb608f694 100644 --- a/xfixes/cursor.c +++ b/xfixes/cursor.c @@ -1045,7 +1045,7 @@ XFixesCursorInit (void) ScreenPtr pScreen = screenInfo.screens[i]; CursorScreenPtr cs; - cs = (CursorScreenPtr) malloc(sizeof (CursorScreenRec)); + cs = (CursorScreenPtr) calloc(1, sizeof (CursorScreenRec)); if (!cs) return FALSE; Wrap (cs, pScreen, CloseScreen, CursorCloseScreen); From 3bbb70a1a7b24d3d1375b20a13db7011cf961c86 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 16 Feb 2011 07:56:58 +1000 Subject: [PATCH 10/10] xfree86: fix up an out-of-date comment. InitInput simply initialises all input devices now. Signed-off-by: Peter Hutterer Reviewed-by: Keith Packard --- hw/xfree86/common/xf86Init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c index a1fda54cd..e664ce451 100644 --- a/hw/xfree86/common/xf86Init.c +++ b/hw/xfree86/common/xf86Init.c @@ -808,7 +808,7 @@ InitInput(int argc, char **argv) GetEventList(&xf86Events); - /* Call the PreInit function for each input device instance. */ + /* Initialize all configured input devices */ for (pDev = xf86ConfigLayout.inputs; pDev && *pDev; pDev++) { /* Replace obsolete keyboard driver with kbd */ if (!xf86NameCmp((*pDev)->driver, "keyboard")) {