From 340de78e26e7837561909ae2a44c2ef85863d87b Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Mon, 17 Jun 2019 17:31:32 +0200 Subject: Keyboard.c: nullify freed pointers While trying to properly free memory allocated by XKB I accidently called nxagentFreeKeyboardDeviceData twice and noticed it would cause a segfault here. As the other pointers are also nullified after being freed let's just do it here, too. --- nx-X11/programs/Xserver/hw/nxagent/Keyboard.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Keyboard.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c index 96b33ea7d..46686b754 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c @@ -1327,14 +1327,18 @@ static int nxagentFreeKeyboardDeviceData(DeviceIntPtr dev) dev->focus = NULL; } - for (k = dev->kbdfeed; k; k = knext) + if (dev->kbdfeed) { - knext = k->next; - #ifdef XKB - if (k->xkb_sli) - XkbFreeSrvLedInfo(k->xkb_sli); - #endif - free(k); + for (k = dev->kbdfeed; k; k = knext) + { + knext = k->next; + #ifdef XKB + if (k->xkb_sli) + XkbFreeSrvLedInfo(k->xkb_sli); + #endif + free(k); + } + dev->kbdfeed = NULL; } #ifdef DEBUG -- cgit v1.2.3 From 4dd1f3cbdff984ff55bc2f88c64b2544c8d88148 Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Mon, 17 Jun 2019 20:25:09 +0200 Subject: Fix memleaks: Free devPrivates of devices on shutdown Fixes these two memory leaks identified by valgrind: ==28336== 32 (8 direct, 24 indirect) bytes in 1 blocks are definitely lost in loss record 180 of 308 ==28336== at 0x48356AF: malloc (vg_replace_malloc.c:298) ==28336== by 0x4837DE7: realloc (vg_replace_malloc.c:826) ==28336== by 0x1AE322: AllocateDevicePrivate (privates.c:439) ==28336== by 0x27527B: XkbSetExtension (xkbActions.c:72) ==28336== by 0x198E9B: _RegisterPointerDevice (devices.c:361) ==28336== by 0x1DBA35: InitInput (Init.c:440) ==28336== by 0x14DBD6: main (main.c:303) ==28336== ==28336== 32 (8 direct, 24 indirect) bytes in 1 blocks are definitely lost in loss record 181 of 308 ==28336== at 0x48356AF: malloc (vg_replace_malloc.c:298) ==28336== by 0x4837DE7: realloc (vg_replace_malloc.c:826) ==28336== by 0x1AE322: AllocateDevicePrivate (privates.c:439) ==28336== by 0x27527B: XkbSetExtension (xkbActions.c:72) ==28336== by 0x198F1B: _RegisterKeyboardDevice (devices.c:384) ==28336== by 0x1DBA3D: InitInput (Init.c:441) ==28336== by 0x14DBD6: main (main.c:303) --- nx-X11/programs/Xserver/hw/nxagent/Keyboard.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Keyboard.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c index 46686b754..c8038955b 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c @@ -1025,11 +1025,18 @@ Reply Total Cached Bits In Bits Out Bits/Reply Ratio break; case DEVICE_CLOSE: - #ifdef TEST fprintf(stderr, "nxagentKeyboardProc: Called for [DEVICE_CLOSE].\n"); #endif + for (int i = 0; i < pDev->nPrivates; i++) + { + free(pDev->devPrivates[i].ptr); + pDev->devPrivates[i].ptr = NULL; + } + free(pDev->devPrivates); + pDev->devPrivates = NULL; + break; } -- cgit v1.2.3 From cb508b2632f661e19b44f1375ddce3ffb415f4c5 Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Sat, 25 May 2019 19:38:14 +0200 Subject: various scope improvements --- nx-X11/programs/Xserver/hw/nxagent/Keyboard.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Keyboard.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c index c8038955b..cff92ec48 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c @@ -1299,8 +1299,6 @@ static int nxagentRestoreKeyboardDeviceData(DeviceIntPtr devBackup, DeviceIntPtr static int nxagentFreeKeyboardDeviceData(DeviceIntPtr dev) { - KbdFeedbackPtr k, knext; - if (!dev) { #ifdef PANIC @@ -1336,7 +1334,7 @@ static int nxagentFreeKeyboardDeviceData(DeviceIntPtr dev) if (dev->kbdfeed) { - for (k = dev->kbdfeed; k; k = knext) + for (KbdFeedbackPtr k = dev->kbdfeed, knext; k; k = knext) { knext = k->next; #ifdef XKB -- cgit v1.2.3 From 6da1066109241d8c84cdb2b4674f4dd2a15c1a9c Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Wed, 19 Jun 2019 23:44:40 +0200 Subject: Keyboard.c: fix three memory leaks ==12976==ERROR: LeakSanitizer: detected memory leaks Direct leak of 6 byte(s) in 1 object(s) allocated from: #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810) #1 0x559ca29c5035 in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:866 #2 0x7a29bff07 () Direct leak of 1 byte(s) in 1 object(s) allocated from: #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810) #1 0x559ca29c509a in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:870 #2 0x7a29bff07 () Direct leak of 1 byte(s) in 1 object(s) allocated from: #0 0x7f510b3ac810 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a810) #1 0x559ca29c507f in nxagentKeyboardProc /home/uli/work/nx/ArcticaProject/nx-libs/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c:869 #2 0x7a29bff07 () SUMMARY: AddressSanitizer: 8 byte(s) leaked in 3 allocation(s). --- nx-X11/programs/Xserver/hw/nxagent/Keyboard.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Keyboard.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c index cff92ec48..b2ae7dfd1 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c @@ -945,6 +945,10 @@ XkbError: { NXShadowInitKeymap(&(pDev->key->curKeySyms)); } + + free(rules); + free(variant); + free(options); } if (xkb) -- cgit v1.2.3 From fd7e1f989b757667e7d7a3f0d08c9ee4b21a3a7c Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Thu, 20 Jun 2019 18:52:31 +0200 Subject: Keyboard.c: rearrange code to make cppcheck happy Otherwise it will (falsely) report "Memory pointed to by 'sessionpath' is freed twice." --- nx-X11/programs/Xserver/hw/nxagent/Keyboard.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Keyboard.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c index b2ae7dfd1..7330784de 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c @@ -1696,7 +1696,10 @@ static char* getKeyboardFilePath(void) free(sessionpath); FatalError("malloc for keyboard file path failed."); } - free(sessionpath); + else + { + free(sessionpath); + } } else { -- cgit v1.2.3 From 2bb498a4c767b7d12db84e59b77020bcd70a057c Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Fri, 21 Jun 2019 11:38:39 +0200 Subject: Keyboard.c: fix another cppcheck finding [Keyboard.c:559]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour --- nx-X11/programs/Xserver/hw/nxagent/Keyboard.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Keyboard.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c index 7330784de..8ff4528a1 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c @@ -555,8 +555,9 @@ void nxagentChangeKeyboardControl(DeviceIntPtr pDev, KeybdCtrl *ctrl) for (int i = 1; i <= 32; i++) { + unsigned int mask = (unsigned int)1 << (i - 1); values.led = i; - values.led_mode = (ctrl->leds & (1 << (i - 1))) ? LedModeOn : LedModeOff; + values.led_mode = (ctrl->leds & mask) ? LedModeOn : LedModeOff; XChangeKeyboardControl(nxagentDisplay, value_mask, &values); } -- cgit v1.2.3 From 364035c00258c17924174dac7921b5a8e68e6459 Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Fri, 21 Jun 2019 11:39:03 +0200 Subject: Keyboard.c: use existing define instead of hardcoced value --- nx-X11/programs/Xserver/hw/nxagent/Keyboard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nx-X11/programs/Xserver/hw/nxagent/Keyboard.c') diff --git a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c index 8ff4528a1..4952c0197 100644 --- a/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c +++ b/nx-X11/programs/Xserver/hw/nxagent/Keyboard.c @@ -553,7 +553,7 @@ void nxagentChangeKeyboardControl(DeviceIntPtr pDev, KeybdCtrl *ctrl) value_mask = KBLed | KBLedMode; - for (int i = 1; i <= 32; i++) + for (int i = 1; i <= XkbNumIndicators; i++) { unsigned int mask = (unsigned int)1 << (i - 1); values.led = i; -- cgit v1.2.3