fix: make the behaviour of methods which set hid_error more consistent #425

Closed

Julusian wants to merge 1 commit from fix/global-error-consistency into master

Julusian commented

2022-06-11 11:48:00 -05:00

(Migrated from github.com)

From reading through the code I found that the behaviour of hid_error() was rather inconsistent at times.

The windows implementation would only set the device error if it encountered an error, and would never clear the last error.
The linux hidraw implementation would sometimes clear it, but not always.

From reading through the code I found that the behaviour of `hid_error()` was rather inconsistent at times. The windows implementation would only set the device error if it encountered an error, and would never clear the last error. The linux hidraw implementation would sometimes clear it, but not always.

Julusian commented

2022-06-11 11:52:03 -05:00

(Migrated from github.com)

I suppose that it could be fine as it, as you know that there is a new error from the return code, so maybe none of this is necessary other than the docs fix

Youw commented

2022-06-12 12:27:40 -05:00

(Migrated from github.com)

You are right, there is so much wrong about hid_error in general.
Starting with the inconsistency in the documentation and implementation, ending up with real thread-safety issues (despite the documentation).

We've introduced a lot problems by trying to make an improvement.
We tried to make hid_error mimic the errno behavior, on the thread-safety part, but that was practically impossible to do so for a few reasons:

hid_error has a per-device version of the function, and the device (struct) member cannot contain thread-local members;
unlike errno, which is an int - hid_error is a dynamically allocated string, which has to be freed by each thread, where the error has been happening, and that is not true for hidraw (the only backend that has global errors implementation);

Let me finally start making it right.

You are right, there is so much wrong about `hid_error` in general. Starting with the inconsistency in the documentation and implementation, ending up with real thread-safety issues (despite the documentation). We've introduced a lot problems by trying to make an [improvement](https://github.com/libusb/hidapi/pull/23). We tried to make `hid_error` mimic the [`errno`](https://man7.org/linux/man-pages/man3/errno.3.html) behavior, on the thread-safety part, but that was practically impossible to do so for a few reasons: - hid_error has a per-device version of the function, and the device (struct) member cannot contain thread-local members; - unlike `errno`, which is an `int` - hid_error is a dynamically allocated string, which has to be `free`d by each thread, where the error has been happening, and that is not true for `hidraw` (the only backend that has global errors implementation); Let me finally start making it [right](https://github.com/libusb/hidapi/tree/hid_error).