I found that in function hid_is_collection the third parameter
of hid_start_parse call is enum item 'hid_input'. I think
it should be a zero value bitmap here, although the value of hid_input currently
is 0. It makes more sense.
Details
- Reviewers
• hselasky wulf delphij - Commits
- rG6ed3b9ca25ba: hid: fix typo in hid_is_collection
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
sys/dev/hid/hid.c | ||
---|---|---|
802 | If I am understanding the code correctly (disclaimer: I am not familiar with this particular module so I could well be wrong), it appears like this should really be 1 << hid_input here. Passing 0 worked because when kind is collection, the kindset is blindly ignored, but for consistency with all other usages in the tree, I still think it's better to pass 1 << hid_input here. BTW. despite documented as allowing a set of bits being passed in kindset, the code would fail when multiple bits are passed to hid_start_parse. It wasn't clear if that's a deliberate choice, or simply that was not being implemented, but the inconsistency of how kindset was kinda confusing. |
sys/dev/hid/hid.c | ||
---|---|---|
802 | Passing 0 is correct for our simple hid_is_collection() implementation. 1 << hid_input is required for more complex NetBSD version which tracks for input usage presence within a collection. See So we should replace hid_input with 0 and leave while loop as is or replace hid_input with 1 << hid_input and import NetBSD code. |
sys/dev/hid/hid.c | ||
---|---|---|
802 |
I think hid_start_parse is for calling hid_get_item in a subsequent loop later. What kind of item we want to get in hid_get_item must be reflected in the third parameter kindset of hid_start_parse. In addition, items of type hid_collection/hid_endcollection will be returned regardless of the kindset.
Maybe simply that was not being implemented in sys/dev/hid/hid.c, when multiple bits are passed to hid_start_parse, hid_get_item can't get item position correctly, because pos in struct hid_data can't handle multiple kinds, functions in lib/libusbhid/parse.c work the same as described in the USBHID(3). |