Date: Thu, 20 Sep 2012 11:46:55 GMT From: Vitaly Magerya <vmagerya@gmail.com> To: freebsd-gnats-submit@FreeBSD.org Subject: usb/171810: [patch] make hid_start_parse(3) respect report ID argument Message-ID: <201209201146.q8KBktFg062349@red.freebsd.org> Resent-Message-ID: <201209201150.q8KBoB2O009218@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 171810
>Category: usb
>Synopsis: [patch] make hid_start_parse(3) respect report ID argument
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-usb
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: change-request
>Submitter-Id: current-users
>Arrival-Date: Thu Sep 20 11:50:11 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator: Vitaly Magerya
>Release:
>Organization:
>Environment:
>Description:
Currently hid_start_parse(3) ignores it's 3-rd parameter -- the
report ID, so subsequent calls to hid_get_item(3) return items
of all report IDs, not just the one requested. This also affects
hid_locate(3), which effectively ignores requested report ID too.
This is different from how it works on NetBSD and OpenBSD: their
libusbhid(3) implementations only return items of the requested
report ID, unless it is -1, in which case items of all report
IDs are returned. In fact our own usbhidctl(1) always uses -1
as the report ID argument, and usbhidaction(1) has an undocumented
option '-r' to use report ID other than -1 -- which doesn't seem
to work at the moment (I did not test it, but it appears that
way from the code).
In short, I propose to follow NetBSD and OpenBSD and respect id
argument of hid_start_parse(3).
Note: we had the same API before revision 205728, which says
"This merge does not change any API", so it seems this was just
overlooked.
>How-To-Repeat:
>Fix:
Patch attached with submission follows:
Index: usbhid.3
===================================================================
--- usbhid.3 (revision 240744)
+++ usbhid.3 (working copy)
@@ -144,16 +144,15 @@
.Ss Descriptor Parsing Functions
To parse the report descriptor the
.Fn hid_start_parse
-function should be called with a report descriptor and a set that
-describes which items that are interesting.
+function should be called with a report descriptor, a set that
+describes which items that are interesting, and the desired report
+ID (or -1 to obtain items of all report IDs).
The set is obtained by OR-ing together values
.Fa "(1 << k)"
where
.Fa k
is an item of type
.Vt hid_kind_t .
-The report ID (if present) is given by
-.Fa id .
The function returns
.Dv NULL
if the initialization fails, otherwise an opaque value to be used
Index: descr.c
===================================================================
--- descr.c (revision 240744)
+++ descr.c (working copy)
@@ -68,7 +68,7 @@
if ((rep = hid_get_report_desc(fd)) == NULL)
goto use_ioctl;
kindset = 1 << hid_input | 1 << hid_output | 1 << hid_feature;
- for (d = hid_start_parse(rep, kindset, 0); hid_get_item(d, &h); ) {
+ for (d = hid_start_parse(rep, kindset, -1); hid_get_item(d, &h); ) {
/* Return the first report ID we met. */
if (h.report_ID != 0) {
temp = h.report_ID;
Index: parse.c
===================================================================
--- parse.c (revision 240744)
+++ parse.c (working copy)
@@ -70,6 +70,7 @@
uint8_t iusage; /* current "usages_min/max" index */
uint8_t ousage; /* current "usages_min/max" offset */
uint8_t susage; /* usage set flags */
+ uint32_t reportid; /* requested report ID */
};
/*------------------------------------------------------------------------*
@@ -149,7 +150,7 @@
* hid_start_parse
*------------------------------------------------------------------------*/
hid_data_t
-hid_start_parse(report_desc_t d, int kindset, int id __unused)
+hid_start_parse(report_desc_t d, int kindset, int id)
{
struct hid_data *s;
@@ -158,6 +159,7 @@
s->start = s->p = d->data;
s->end = d->data + d->size;
s->kindset = kindset;
+ s->reportid = id;
return (s);
}
@@ -207,8 +209,8 @@
/*------------------------------------------------------------------------*
* hid_get_item
*------------------------------------------------------------------------*/
-int
-hid_get_item(hid_data_t s, hid_item_t *h)
+static int
+hid_get_item_raw(hid_data_t s, hid_item_t *h)
{
hid_item_t *c;
unsigned int bTag, bType, bSize;
@@ -509,6 +511,19 @@
}
int
+hid_get_item(hid_data_t s, hid_item_t *h)
+{
+ int r;
+
+ for (;;) {
+ r = hid_get_item_raw(s, h);
+ if (r <= 0 || s->reportid == -1 || h->report_ID == s->reportid)
+ break;
+ }
+ return (r);
+}
+
+int
hid_report_size(report_desc_t r, enum hid_kind k, int id)
{
struct hid_data *d;
@@ -523,7 +538,7 @@
memset(&h, 0, sizeof h);
for (d = hid_start_parse(r, 1 << k, id); hid_get_item(d, &h); ) {
- if ((h.report_ID == id || id < 0) && h.kind == k) {
+ if (h.kind == k) {
/* compute minimum */
if (lpos > h.pos)
lpos = h.pos;
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201209201146.q8KBktFg062349>