vlovich/hidapi — comment on commit 0dc21838c4c2112faaeee2b8cb4f2c65ba5fbe1d
File:  line 0
By vlovich:

It's accurate for the linux kernel where there's access to
platform-specific memory barriers, which are not directly available.
That being said, you are correct, volatile isn't necessary in the
struct & I rechecked the accesses of the various variables; only
certain accesses of disconnected seem like they perhaps require a
volatile cast; everything else does appear to be surrounded by calls
to pthread functions that imply memory barriers (pthread function call
prevents compiler doing speculative reads & the function itself is a
memory fence).

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_11

You are correct - dev_handle probably can't be null in read. I can't
at this point recall whether that check was in response to an actual
problem or a perceived one (I'm guessing the latter).

The IOHIDDeviceClose does need to be removed - I believe I was seeing
scenarios where it was being called twice causing a crash.

On Fri, Dec 30, 2011 at 2:47 PM, Alan Ott

wrote:
> Some people think volatile is bad (search for "volatile considered evil"). I don't know enough about it to make an assertion about it. What are your thoughts?
>
> What case are you seeing where dev->device_handle could possibly be NULL in read_thread() ? To me it seems to go as follows:
> 1. User calls hid_close()
> 2. hid_close blocks waiting for read_thread() to finish.
> 3. read_thread() closes dev->device_handle (line 683) and then finishes
> 4. hid_close() closes dev->device_handle and sets it to NULL.
>
> It seems to me like maybe the IOHIDDeviceClose() can be removed from hid_close(), but I'm not seeing any reason dev->device_handle can be NULL in read_thread(), or any way that it can be anything other than already closed in hid_close().
>
> Also, in hid_close, you change
>
> ```
> IOHIDDeviceRegisterInputReportCallback(dev->device_handle, dev->input_report_buf, dev->max_input_report_len, NULL, dev);
> ```
>
> to
>
> ```
> IOHIDDeviceRegisterInputReportCallback(dev->device_handle, NULL, 0, NULL, NULL);
> ```
>
> There is a reason it was like this. It had to do with opening/closing the device repeatedly.....
> I found the commit. Take a look at the commit message here:
> https://github.com/signal11/hidapi/commit/51c18081bf37af2e40be071aaa892505bc1fda08
>
> Alan.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/vlovich/hidapi/commit/0dc21838c4c2112faaeee2b8cb4f2c65ba5fbe1d#commitcomment-828661