| Summary: | Crash when running with -DHIDAPI=ON | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Sven-Hendrik Haase <svenstaro> |
| Component: | joystick | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | meyraud705, sezeroz, stfx, svenstaro |
| Version: | HG 2.0 | Keywords: | target-2.0.14 |
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: |
fix error checking and initialisation in SDL_hidapi.c
fix error checking and initialisation in SDL_hidapi.c (no log) |
||
Some more system information: Arch Linux cmake 3.17.3 gcc 10.1.0 Our downstream bug report about this: https://bugs.archlinux.org/task/67174 Created attachment 4402 [details]
fix error checking and initialisation in SDL_hidapi.c
SDL dynamically loads libusb but does not check the return value of 'SDL_LoadFunction'.
Also libusb is loaded and initialized several time because 'SDL_hidapi_wasinit' is never set to true.
I made a patch if you want to test:
- check that 'hid_init' is called once and only once,
- check return value of 'hid_init',
- check return value of 'SDL_LoadFunction',
- check return value of 'SDL_malloc',
- add some debug logging.
(In reply to meyraud705 from comment #3) > Created attachment 4402 [details] > fix error checking and initialisation in SDL_hidapi.c > > SDL dynamically loads libusb but does not check the return value of > 'SDL_LoadFunction'. > > Also libusb is loaded and initialized several time because > 'SDL_hidapi_wasinit' is never set to true. > > I made a patch if you want to test: > - check that 'hid_init' is called once and only once, > - check return value of 'hid_init', > - check return value of 'SDL_LoadFunction', > - check return value of 'SDL_malloc', > - add some debug logging. Doesn't compile for me: FAILED: CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o /usr/bin/cc -DSDL2_EXPORTS -DSDL_USE_IME -DUSING_GENERATED_CONFIG_H -Iinclude -I../include -Iwayland-generated-protocols -I/usr/include/libdrm -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include -I/usr/include/ibus-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/libmount -I/usr/include/blkid -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -idirafter /build/sdl2/src/SDL2-2.0.12/src/video/khronos -I/usr/include/libusb-1.0 -I/build/sdl2/src/SDL2-2.0.12/src/hidapi/hidapi -DHAVE_LINUX_VERSION_H -I/usr/include/libdrm -I/usr/include -D_REENTRANT -msse3 -msse2 -msse -m3dnow -mmmx -Wshadow -fvisibility=hidden -Wdeclaration-after-statement -Werror=declaration-after-statement -fno-strict-aliasing -Wall -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fPIC -MD -MT CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o -MF CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o.d -o CMakeFiles/SDL2.dir/src/hidapi/SDL_hidapi.c.o -c ../src/hidapi/SDL_hidapi.c In file included from ../src/hidapi/../SDL_internal.h:45, from ../src/hidapi/SDL_hidapi.c:31: ../src/hidapi/SDL_hidapi.c: In function ‘hid_init’: ../src/hidapi/../dynapi/SDL_dynapi_overrides.h:35:21: warning: implicit declaration of function ‘SDL_LogWarn_REAL’; did you mean ‘SDL_CondWait_REAL’? [-Wimplicit-function-declaration] 35 | #define SDL_LogWarn SDL_LogWarn_REAL | ^~~~~~~~~~~~~~~~ ../src/hidapi/SDL_hidapi.c:501:13: note: in expansion of macro ‘SDL_LogWarn’ 501 | SDL_LogWarn(SDL_LOG_CATEGORY_INPUT, SDL_LIBUSB_DYNAMIC " found but could not load function."); | ^~~~~~~~~~~ ../src/hidapi/SDL_hidapi.c:501:25: error: ‘SDL_LOG_CATEGORY_INPUT’ undeclared (first use in this function) 501 | SDL_LogWarn(SDL_LOG_CATEGORY_INPUT, SDL_LIBUSB_DYNAMIC " found but could not load function."); | ^~~~~~~~~~~~~~~~~~~~~~ Created attachment 4403 [details]
fix error checking and initialisation in SDL_hidapi.c (no log)
Same patch with problematic line commented out (you will not get logging in case SDL_LoadFunction fail).
I don't understand why it doesn't compile for you. SDL_hidapi.c includes SDL_internals.h which include SDL_Log.h, and SDL_log.h defines SDL_LOG_CATEGORY_INPUT.
That seems to be working well for me, cool! A user reports that while it doesn't crash, it also doesn't work. I'll try to get them to participate here. This looks like an important patch to make using HIDAPI less risky Patch added, thanks! https://hg.libsdl.org/SDL/rev/26a87784645c |
Applications that init the joystick subsystem crash after I use compile my SDL with -DHIDAPI=ON. Example running blobby volley (SDL2) with on recent tip with -DHIDAPI=ON: #0 0x0000000000000000 in () #1 0x00007ffff7e3e555 in LIBUSB_hid_init () at ../src/hidapi/libusb/hid.c:460 #2 hid_init () at ../src/hidapi/SDL_hidapi.c:491 #3 0x00007ffff7e40435 in hid_init () at ../src/hidapi/SDL_hidapi.c:454 #4 0x00007ffff7e3cb99 in HIDAPI_JoystickInit () at ../src/joystick/hidapi/SDL_hidapijoystick.c:582 #5 0x00007ffff7e01eb4 in SDL_JoystickInit () at ../src/joystick/SDL_joystick.c:238 #6 0x00007ffff7d2a2b5 in SDL_InitSubSystem_REAL (flags=16944) at ../src/SDL.c:234 #7 0x0000555555669348 in main () I compile my SDL2 like this: cmake . \ -Bbuild \ -GNinja \ -DCMAKE_INSTALL_PREFIX=/usr \ -DSDL_STATIC=OFF \ -DSDL_DLOPEN=ON \ -DARTS=OFF \ -DESD=OFF \ -DNAS=OFF \ -DALSA=ON \ -DHIDAPI=ON \ -DPULSEAUDIO_SHARED=ON \ -DVIDEO_WAYLAND=ON \ -DRPATH=OFF \ -DCLOCK_GETTIME=ON \ -DJACK_SHARED=ON ninja -C build