>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:
_______________________________________________
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"

Reply via email to