libusbhid bug?

[Stefan `Sec` Zehl]

Hi,

I've been playing around with a Bluetooth HID device. Bluetooth uses the
same HID format as USB, so libusbhid is used to parse these messages.

While debugging, I've found what I think is a mistake in parsing
"Array"-type input messages:

In my HID descriptor, there is the following snippet:

| 95 06   |    Report Count(6)
| 75 08   |    Report Size(8)
| 15 00   |    Logical Minimum(0)
| 26 a4 00|    Logical Maximum(164)
| 05 07   |    Usage Page(Key Codes)
| 19 00   |    Usage Minimum(0x00)
| 29 a4   |    Usage Maximum(0xa4)
| 81 00   |    Input(Data,Array,Absolute)

The current usbhid only parses one item of Report_Size (8bit). According
to the spec it should parse Report_Count items:

| An array provides an alternate means for
| describing the data returned from a group of
| buttons. Arrays are more efficient, if less flexible
| than variable items. Rather than returning a single
| bit for each button in the group, an array returns an
| index in each field that corresponds to the pressed
| button (like keyboard scan codes). An out-of range
| value in and array field is considered no controls
| asserted. Buttons or keys in an array that are
| simultaneously pressed need to be reported in
| multiple fields. Therefore, the number of fields in
| an array input item (Report Count) dictates the
| maximum number of simultaneous controls that
| can be reported. A keyboard could report up to
| three simultaneous keys using an array with three
| 8-bit fields (Report Size = 8, Report Count = 3).

I have created a patch which changes that behaviour, and attached it
below. My sample code works fine with these changes, but as I don't have
an USB Keyboard, it would be great if someone could check that this
doesn't break anything.

--- parse.c.org	Wed Feb 11 22:09:13 2004
+++ parse.c	Tue May 15 01:26:08 2007
@@ -54,6 +54,7 @@
 	int multimax;
 	int kindset;
 	int reportid;
+	int isarray;
 
 	/*
 	 * The start of collection item has no report ID set, so save
@@ -100,6 +101,7 @@
 	s->kindset = kindset;
 	s->reportid = id;
 	s->hassavedcoll = 0;
+	s->isarray = 0;
 	return (s);
 }
 
@@ -182,6 +184,16 @@
 			hid_clear_local(c);
 		}
 	}
+	if(s->isarray) {
+		REPORT_SAVED_COLL;
+		*h = *c;
+		h->next = 0;
+		h->pos = s->kindpos[c->kind];
+		s->kindpos[c->kind] += c->report_size ;
+		if(--s->isarray ==0)
+			hid_clear_local(c);
+		return(1);
+	};
 	for (;;) {
 		p = s->p;
 		if (p >= s->end)
@@ -262,14 +274,9 @@
 				} else {
 					if (s->minset)
 						c->usage = c->usage_minimum;
-					*h = *c;
-					h->next = 0;
-					h->pos = s->kindpos[c->kind];
-					s->kindpos[c->kind] +=
-					    c->report_size * c->report_count;
-					hid_clear_local(c);
 					s->minset = 0;
-					return (1);
+					s->isarray=c->report_count;
+					goto top;
 				}
 			case 9:		/* Output */
 				retkind = hid_output;

CU,
    Sec
-- 
  "The General who in a hundred battles is always victorious is not as
  great as the one who achieves his objectives without fighting."
                                             -- Sun Tzu