We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4738 - joystick: Ensure HIDAPI is initialized before calling it
Summary: joystick: Ensure HIDAPI is initialized before calling it
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: joystick (show other bugs)
Version: HG 2.0
Hardware: x86_64 Linux
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-25 13:12 UTC by Andrew Eikum
Modified: 2019-07-31 17:21 UTC (History)
2 users (show)

See Also:

Attachments
[PATCH] joystick/hidapi: Initialize hidapi in HIDAPI_IsDevicePresent (1.19 KB, patch)
2019-07-25 13:12 UTC, Andrew Eikum 		Details | Diff
joystick: Ensure HIDAPI is initialized before calling it (1.78 KB, patch)
2019-07-31 16:16 UTC, Andrew Eikum 		Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Eikum 2019-07-25 13:12:29 UTC 
Created attachment 3896 [details]
[PATCH] joystick/hidapi: Initialize hidapi in HIDAPI_IsDevicePresent

The Linux joystick backend calls HIDAPI_IsDevicePresent during its Init method.
This occurs _before_ the HIDAPI joystick backend's Init method is called, in
other words, before hid_init() is called. Invoking hidapi methods before
hid_init() is invalid. Unlike the SDL JoystickInit methods, hid_init() is safe
to call more than once, so let's do that twice in the HIDAPI joystick code.
Comment 1 Andrew Eikum 2019-07-25 13:19:08 UTC 
A better solution might be to avoid calling into other joystick backends from JoystickInit methods, but that's beyond me.
Comment 2 Cameron Gutman 2019-07-25 17:06:38 UTC 
I don't think calling hid_init() is required in our scenario.

The function is documented as follows:

		/** @brief Initialize the HIDAPI library.
			This function initializes the HIDAPI library. Calling it is not
			strictly necessary, as it will be called automatically by
			hid_enumerate() and any of the hid_open_*() functions if it is
			needed.  This function should be called at the beginning of
			execution however, if there is a chance of HIDAPI handles
			being opened by different threads simultaneously.
			
			@ingroup API
			@returns
				This function returns 0 on success and -1 on error.
		*/
		int HID_API_EXPORT HID_API_CALL hid_init(void);

So calling HIDAPI_IsDevicePresent() from an arbitrary JoystickInit() is safe because it's guaranteed to be on the same thread that will later call HIDAPI's JoystickInit().
Comment 3 Andrew Eikum 2019-07-25 17:36:15 UTC 
Oh! Good find! That shows this is a bug in the multiple backends code from Bug 4554. I'll make a note over there.
Comment 4 Andrew Eikum 2019-07-31 16:16:43 UTC 
Created attachment 3912 [details]
joystick: Ensure HIDAPI is initialized before calling it

This regressed because of d9699df941ac "Don't call hid_enumerate() if the HIDAPI drivers are all disabled". I think it's clear that calling into HIDAPI before it is initialized is going to continue to cause problems. Here's a patch that just initializes HIDAPI before we call into it, and also makes HIDAPI's Init method safe to call more than once.
Comment 5 Andrew Eikum 2019-07-31 16:17:08 UTC 
Reopening for review