From 2c204ab52ecaa51bfd32b777aef715dae937f009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Camilla=20L=C3=B6wy?= Date: Thu, 5 May 2022 22:23:12 +0200 Subject: [PATCH] Fix joystick user pointer NULL during disconnect The joystick code did not distinguish between the allocation status of the GLFW joystick object and whether it is connection to an OS level joystick object. These are now tracked separately. Fixes #2092 --- CONTRIBUTORS.md | 1 + README.md | 1 + src/cocoa_joystick.m | 15 ++++++++------- src/input.c | 33 +++++++++++++++++++-------------- src/internal.h | 3 ++- src/linux_joystick.c | 8 ++++---- src/win32_joystick.c | 10 +++++----- 7 files changed, 40 insertions(+), 31 deletions(-) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 030abf7c..23d1418e 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -137,6 +137,7 @@ video tutorials. - Kenneth Miller - Bruce Mitchener - Jack Moffitt + - Ravi Mohan - Jeff Molofee - Alexander Monakov - Pierre Morel diff --git a/README.md b/README.md index 033f1bf2..f189ba20 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,7 @@ information on what to include when reporting a bug. - Bugfix: Native access functions for context handles did not check that the API matched - Bugfix: `glfwMakeContextCurrent` would access TLS slot before initialization - Bugfix: `glfwSetGammaRamp` could emit `GLFW_INVALID_VALUE` before initialization + - Bugfix: `glfwGetJoystickUserPointer` returned `NULL` during disconnection (#2092) - [Win32] Added the `GLFW_WIN32_KEYBOARD_MENU` window hint for enabling access to the window menu - [Win32] Added a version info resource to the GLFW DLL diff --git a/src/cocoa_joystick.m b/src/cocoa_joystick.m index e09e1efa..7c90b4c1 100644 --- a/src/cocoa_joystick.m +++ b/src/cocoa_joystick.m @@ -96,8 +96,7 @@ static CFComparisonResult compareElements(const void* fp, // static void closeJoystick(_GLFWjoystick* js) { - if (!js->present) - return; + _glfwInputJoystick(js, GLFW_DISCONNECTED); for (int i = 0; i < CFArrayGetCount(js->ns.axes); i++) _glfw_free((void*) CFArrayGetValueAtIndex(js->ns.axes, i)); @@ -112,7 +111,6 @@ static void closeJoystick(_GLFWjoystick* js) CFRelease(js->ns.hats); _glfwFreeJoystick(js); - _glfwInputJoystick(js, GLFW_DISCONNECTED); } // Callback for user-initiated joystick addition @@ -289,9 +287,9 @@ static void removeCallback(void* context, { for (int jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) { - if (_glfw.joysticks[jid].ns.device == device) + if (_glfw.joysticks[jid].connected && _glfw.joysticks[jid].ns.device == device) { - closeJoystick(_glfw.joysticks + jid); + closeJoystick(&_glfw.joysticks[jid]); break; } } @@ -382,7 +380,10 @@ GLFWbool _glfwInitJoysticksCocoa(void) void _glfwTerminateJoysticksCocoa(void) { for (int jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) - closeJoystick(_glfw.joysticks + jid); + { + if (_glfw.joysticks[jid].connected) + closeJoystick(&_glfw.joysticks[jid]); + } if (_glfw.ns.hidManager) { @@ -455,7 +456,7 @@ int _glfwPollJoystickCocoa(_GLFWjoystick* js, int mode) } } - return js->present; + return js->connected; } const char* _glfwGetMappingNameCocoa(void) diff --git a/src/input.c b/src/input.c index e0a12cce..01da7d65 100644 --- a/src/input.c +++ b/src/input.c @@ -377,6 +377,11 @@ void _glfwInputJoystick(_GLFWjoystick* js, int event) { const int jid = (int) (js - _glfw.joysticks); + if (event == GLFW_CONNECTED) + js->connected = GLFW_TRUE; + else if (event == GLFW_DISCONNECTED) + js->connected = GLFW_FALSE; + if (_glfw.callbacks.joystick) _glfw.callbacks.joystick(jid, event); } @@ -442,7 +447,7 @@ _GLFWjoystick* _glfwAllocJoystick(const char* name, for (jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) { - if (!_glfw.joysticks[jid].present) + if (!_glfw.joysticks[jid].allocated) break; } @@ -450,7 +455,7 @@ _GLFWjoystick* _glfwAllocJoystick(const char* name, return NULL; js = _glfw.joysticks + jid; - js->present = GLFW_TRUE; + js->allocated = GLFW_TRUE; js->axes = _glfw_calloc(axisCount, sizeof(float)); js->buttons = _glfw_calloc(buttonCount + (size_t) hatCount * 4, 1); js->hats = _glfw_calloc(hatCount, 1); @@ -972,7 +977,7 @@ GLFWAPI int glfwJoystickPresent(int jid) return GLFW_FALSE; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return GLFW_FALSE; return _glfw.platform.pollJoystick(js, _GLFW_POLL_PRESENCE); @@ -1000,7 +1005,7 @@ GLFWAPI const float* glfwGetJoystickAxes(int jid, int* count) return NULL; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return NULL; if (!_glfw.platform.pollJoystick(js, _GLFW_POLL_AXES)) @@ -1032,7 +1037,7 @@ GLFWAPI const unsigned char* glfwGetJoystickButtons(int jid, int* count) return NULL; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return NULL; if (!_glfw.platform.pollJoystick(js, _GLFW_POLL_BUTTONS)) @@ -1068,7 +1073,7 @@ GLFWAPI const unsigned char* glfwGetJoystickHats(int jid, int* count) return NULL; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return NULL; if (!_glfw.platform.pollJoystick(js, _GLFW_POLL_BUTTONS)) @@ -1097,7 +1102,7 @@ GLFWAPI const char* glfwGetJoystickName(int jid) return NULL; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return NULL; if (!_glfw.platform.pollJoystick(js, _GLFW_POLL_PRESENCE)) @@ -1125,7 +1130,7 @@ GLFWAPI const char* glfwGetJoystickGUID(int jid) return NULL; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return NULL; if (!_glfw.platform.pollJoystick(js, _GLFW_POLL_PRESENCE)) @@ -1144,7 +1149,7 @@ GLFWAPI void glfwSetJoystickUserPointer(int jid, void* pointer) _GLFW_REQUIRE_INIT(); js = _glfw.joysticks + jid; - if (!js->present) + if (!js->allocated) return; js->userPointer = pointer; @@ -1160,7 +1165,7 @@ GLFWAPI void* glfwGetJoystickUserPointer(int jid) _GLFW_REQUIRE_INIT_OR_RETURN(NULL); js = _glfw.joysticks + jid; - if (!js->present) + if (!js->allocated) return NULL; return js->userPointer; @@ -1230,7 +1235,7 @@ GLFWAPI int glfwUpdateGamepadMappings(const char* string) for (jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) { _GLFWjoystick* js = _glfw.joysticks + jid; - if (js->present) + if (js->connected) js->mapping = findValidMapping(js); } @@ -1256,7 +1261,7 @@ GLFWAPI int glfwJoystickIsGamepad(int jid) return GLFW_FALSE; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return GLFW_FALSE; if (!_glfw.platform.pollJoystick(js, _GLFW_POLL_PRESENCE)) @@ -1284,7 +1289,7 @@ GLFWAPI const char* glfwGetGamepadName(int jid) return NULL; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return NULL; if (!_glfw.platform.pollJoystick(js, _GLFW_POLL_PRESENCE)) @@ -1319,7 +1324,7 @@ GLFWAPI int glfwGetGamepadState(int jid, GLFWgamepadstate* state) return GLFW_FALSE; js = _glfw.joysticks + jid; - if (!js->present) + if (!js->connected) return GLFW_FALSE; if (!_glfw.platform.pollJoystick(js, _GLFW_POLL_ALL)) diff --git a/src/internal.h b/src/internal.h index 7babe7e8..be92b03a 100644 --- a/src/internal.h +++ b/src/internal.h @@ -634,7 +634,8 @@ struct _GLFWmapping // struct _GLFWjoystick { - GLFWbool present; + GLFWbool allocated; + GLFWbool connected; float* axes; int axisCount; unsigned char* buttons; diff --git a/src/linux_joystick.c b/src/linux_joystick.c index da04e9c3..2fbd689e 100644 --- a/src/linux_joystick.c +++ b/src/linux_joystick.c @@ -128,7 +128,7 @@ static GLFWbool openJoystickDevice(const char* path) { for (int jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) { - if (!_glfw.joysticks[jid].present) + if (_glfw.joysticks[jid].connected) continue; if (strcmp(_glfw.joysticks[jid].linjs.path, path) == 0) return GLFW_FALSE; @@ -245,9 +245,9 @@ static GLFWbool openJoystickDevice(const char* path) // static void closeJoystick(_GLFWjoystick* js) { + _glfwInputJoystick(js, GLFW_DISCONNECTED); close(js->linjs.fd); _glfwFreeJoystick(js); - _glfwInputJoystick(js, GLFW_DISCONNECTED); } // Lexically compare joysticks by name; used by qsort @@ -366,7 +366,7 @@ void _glfwTerminateJoysticksLinux(void) for (int jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) { _GLFWjoystick* js = _glfw.joysticks + jid; - if (js->present) + if (js->connected) closeJoystick(js); } @@ -417,7 +417,7 @@ int _glfwPollJoystickLinux(_GLFWjoystick* js, int mode) handleAbsEvent(js, e.code, e.value); } - return js->present; + return js->connected; } const char* _glfwGetMappingNameLinux(void) diff --git a/src/win32_joystick.c b/src/win32_joystick.c index 7eb9b203..bac53ab3 100644 --- a/src/win32_joystick.c +++ b/src/win32_joystick.c @@ -256,6 +256,8 @@ static GLFWbool supportsXInput(const GUID* guid) // static void closeJoystick(_GLFWjoystick* js) { + _glfwInputJoystick(js, GLFW_DISCONNECTED); + if (js->win32.device) { IDirectInputDevice8_Unacquire(js->win32.device); @@ -263,9 +265,7 @@ static void closeJoystick(_GLFWjoystick* js) } _glfw_free(js->win32.objects); - _glfwFreeJoystick(js); - _glfwInputJoystick(js, GLFW_DISCONNECTED); } // DirectInput device object enumeration callback @@ -357,7 +357,7 @@ static BOOL CALLBACK deviceCallback(const DIDEVICEINSTANCE* di, void* user) for (jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) { js = _glfw.joysticks + jid; - if (js->present) + if (js->connected) { if (memcmp(&js->win32.guid, &di->guidInstance, sizeof(GUID)) == 0) return DIENUM_CONTINUE; @@ -508,7 +508,7 @@ void _glfwDetectJoystickConnectionWin32(void) for (jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) { - if (_glfw.joysticks[jid].present && + if (_glfw.joysticks[jid].connected && _glfw.joysticks[jid].win32.device == NULL && _glfw.joysticks[jid].win32.index == index) { @@ -560,7 +560,7 @@ void _glfwDetectJoystickDisconnectionWin32(void) for (jid = 0; jid <= GLFW_JOYSTICK_LAST; jid++) { _GLFWjoystick* js = _glfw.joysticks + jid; - if (js->present) + if (js->connected) _glfwPollJoystickWin32(js, _GLFW_POLL_PRESENCE); } }