From owner-freebsd-usb@FreeBSD.ORG Thu Sep 20 11:50:14 2012 Return-Path: Delivered-To: freebsd-usb@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 3911710656A9 for ; Thu, 20 Sep 2012 11:50:14 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id DFFA58FC63 for ; Thu, 20 Sep 2012 11:50:11 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q8KBoBZc009219 for ; Thu, 20 Sep 2012 11:50:11 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q8KBoB2O009218; Thu, 20 Sep 2012 11:50:11 GMT (envelope-from gnats) Resent-Date: Thu, 20 Sep 2012 11:50:11 GMT Resent-Message-Id: <201209201150.q8KBoB2O009218@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-usb@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Vitaly Magerya Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 03C28106566B for ; Thu, 20 Sep 2012 11:46:55 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from red.freebsd.org (red.freebsd.org [IPv6:2001:4f8:fff6::22]) by mx1.freebsd.org (Postfix) with ESMTP id CE9CF8FC08 for ; Thu, 20 Sep 2012 11:46:55 +0000 (UTC) Received: from red.freebsd.org (localhost [127.0.0.1]) by red.freebsd.org (8.14.5/8.14.5) with ESMTP id q8KBkt2D062484 for ; Thu, 20 Sep 2012 11:46:55 GMT (envelope-from nobody@red.freebsd.org) Received: (from nobody@localhost) by red.freebsd.org (8.14.5/8.14.5/Submit) id q8KBktFg062349; Thu, 20 Sep 2012 11:46:55 GMT (envelope-from nobody) Message-Id: <201209201146.q8KBktFg062349@red.freebsd.org> Date: Thu, 20 Sep 2012 11:46:55 GMT From: Vitaly Magerya To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: usb/171810: [patch] make hid_start_parse(3) respect report ID argument X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Sep 2012 11:50:14 -0000 >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: