Re: usbhidctl: add -R flag to dump raw report descriptor bytes
On Fri, 28 May 2021 at 21:52:57 -0500, joshua stein wrote:
> > Another approach which keeps the structure opaque is the extend the
> > usbhid(3) API with something like:
> >
> > void
> > hid_get_report_desc_data(report_desc_t d, uint8_t **data, uint32_t
> > *size)
> > {
> > *data = d->data;
> > *size = d->size;
> > }
Here's a new version of the usbhidctl diff using this new usbhid
API, with man page help from jmc.
diff --git usr.bin/usbhidctl/usbhid.c usr.bin/usbhidctl/usbhid.c
index 921f211a280..910263d1392 100644
--- usr.bin/usbhidctl/usbhid.c
+++ usr.bin/usbhidctl/usbhid.c
@@ -755,6 +755,7 @@ usage(void)
fprintf(stderr,
" %s -f device [-t table] -w name=value ...\n",
__progname);
+ fprintf(stderr, " %s -f device -R\n", __progname);
exit(1);
}
@@ -764,16 +765,18 @@ main(int argc, char **argv)
char const *dev;
char const *table;
size_t varnum;
- int aflag, lflag, nflag, rflag, wflag;
- int ch, hidfd;
+ uint32_t repsize;
+ int aflag, lflag, nflag, rflag, Rflag, wflag;
+ int ch, hidfd, x;
+ uint8_t *repdata;
report_desc_t repdesc;
char devnamebuf[PATH_MAX];
struct Susbvar variables[128];
- wflag = aflag = nflag = verbose = rflag = lflag = 0;
+ wflag = aflag = nflag = verbose = rflag = Rflag = lflag = 0;
dev = NULL;
table = NULL;
- while ((ch = getopt(argc, argv, "?af:lnrt:vw")) != -1) {
+ while ((ch = getopt(argc, argv, "?af:lnRrt:vw")) != -1) {
switch (ch) {
case 'a':
aflag = 1;
@@ -790,6 +793,9 @@ main(int argc, char **argv)
case 'r':
rflag = 1;
break;
+ case 'R':
+ Rflag = 1;
+ break;
case 't':
table = optarg;
break;
@@ -807,7 +813,8 @@ main(int argc, char **argv)
}
argc -= optind;
argv += optind;
- if (dev == NULL || (lflag && (wflag || rflag))) {
+ if (dev == NULL || (lflag && (wflag || rflag || Rflag)) ||
+ (rflag && Rflag)) {
/*
* No device specified, or attempting to loop and set
* or dump report at the same time
@@ -942,6 +949,14 @@ main(int argc, char **argv)
if (repdesc == 0)
errx(1, "USB_GET_REPORT_DESC");
+ if (Rflag) {
+ hid_get_report_desc_data(repdesc, , );
+
+ for (x = 0; x < repsize; x++)
+ printf("%s0x%02x", x > 0 ? " " : "", repdata[x]);
+ printf("\n");
+ }
+
if (lflag) {
devloop(hidfd, repdesc, variables, varnum);
/* NOTREACHED */
@@ -951,10 +966,11 @@ main(int argc, char **argv)
/* Report mode header */
printf("Report descriptor:\n");
- devshow(hidfd, repdesc, variables, varnum,
- 1 << hid_input |
- 1 << hid_output |
- 1 << hid_feature);
+ if (!Rflag)
+ devshow(hidfd, repdesc, variables, varnum,
+ 1 << hid_input |
+ 1 << hid_output |
+ 1 << hid_feature);
if (rflag) {
/* Report mode trailer */
diff --git usr.bin/usbhidctl/usbhidctl.1 usr.bin/usbhidctl/usbhidctl.1
index 5b8e59f7bd7..0aa5b54f6f9 100644
--- usr.bin/usbhidctl/usbhidctl.1
+++ usr.bin/usbhidctl/usbhidctl.1
@@ -53,6 +53,9 @@
.Fl f Ar device
.Op Fl t Ar table
.Fl w Ar name Ns = Ns Ar value ...
+.Nm
+.Fl f Ar device
+.Fl R
.Sh DESCRIPTION
.Nm
can be used to output or modify the state of a USB HID (Human Interface
Device).
@@ -88,6 +91,8 @@ Only 'input' items are displayed in this mode.
.It Fl n
Suppress printing of the item name when querying specific items.
Only output the current value.
+.It Fl R
+Dump the raw USB HID report descriptor data as hexadecimal bytes.
.It Fl r
Dump the USB HID report descriptor.
.It Fl t Ar table
Re: usbhidctl: add -R flag to dump raw report descriptor bytes
On Fri, May 28, 2021 at 09:52:57PM -0500, joshua stein wrote:
> On Wed, 26 May 2021 at 08:13:52 +0200, Anton Lindqvist wrote:
> > On Tue, May 25, 2021 at 08:31:14AM +0200, Anton Lindqvist wrote:
> > > On Mon, May 24, 2021 at 09:17:26AM -0500, joshua stein wrote:
> > > > This is useful for parsing the report descriptor with a different
> > > > tool to find issues with our HID parser.
> > > >
> > > > I've found https://eleccelerator.com/usbdescreqparser/ to be
> > > > helpful.
> > > >
> > > >
> > > > diff --git usr.bin/usbhidctl/usbhid.c usr.bin/usbhidctl/usbhid.c
> > > > index 921f211a280..bd0b5da0222 100644
> > > > --- usr.bin/usbhidctl/usbhid.c
> > > > +++ usr.bin/usbhidctl/usbhid.c
> > > > @@ -106,6 +106,11 @@ static struct {
> > > > #define REPORT_MAXVAL 2
> > > > };
> > > >
> > > > +struct report_desc {
> > > > + uint32_t size;
> > > > + uint8_t data[1];
> > > > +};
> > >
> > > This structure is defined in lib/libusbhid/usbvar.h which is not
> > > installed. Maybe it should and avoid this repetition and potential ABI
> > > breaks (quite unlikely but still)?
> >
> > Another approach which keeps the structure opaque is the extend the
> > usbhid(3) API with something like:
> >
> > void
> > hid_get_report_desc_data(report_desc_t d, uint8_t **data, uint32_t
> > *size)
> > {
> > *data = d->data;
> > *size = d->size;
> > }
>
> Here's a version of that:
libusbhid needs a minor bump:
Index: lib/libusbhid/shlib_version
===
RCS file: /cvs/src/lib/libusbhid/shlib_version,v
retrieving revision 1.8
diff -u -p -r1.8 shlib_version
--- lib/libusbhid/shlib_version 12 May 2014 17:03:28 - 1.8
+++ lib/libusbhid/shlib_version 30 May 2021 07:04:33 -
@@ -1,2 +1,2 @@
major=7
-minor=0
+minor=1
Index: distrib/sets/lists/base/mi
===
RCS file: /cvs/src/distrib/sets/lists/base/mi,v
retrieving revision 1.1022
diff -u -p -r1.1022 mi
--- distrib/sets/lists/base/mi 27 May 2021 05:51:50 - 1.1022
+++ distrib/sets/lists/base/mi 30 May 2021 07:04:51 -
@@ -675,7 +675,7 @@
./usr/lib/libtermcap.so.14.0
./usr/lib/libtermlib.so.14.0
./usr/lib/libtls.so.21.0
-./usr/lib/libusbhid.so.7.0
+./usr/lib/libusbhid.so.7.1
./usr/lib/libutil.so.15.0
./usr/lib/libz.so.5.0
./usr/lib/locate
Re: usbhidctl: add -R flag to dump raw report descriptor bytes
On Fri, May 28, 2021 at 09:52:57PM -0500, joshua stein wrote:
> On Wed, 26 May 2021 at 08:13:52 +0200, Anton Lindqvist wrote:
> > On Tue, May 25, 2021 at 08:31:14AM +0200, Anton Lindqvist wrote:
> > > On Mon, May 24, 2021 at 09:17:26AM -0500, joshua stein wrote:
> > > > This is useful for parsing the report descriptor with a different
> > > > tool to find issues with our HID parser.
> > > >
> > > > I've found https://eleccelerator.com/usbdescreqparser/ to be
> > > > helpful.
> > > >
> > > >
> > > > diff --git usr.bin/usbhidctl/usbhid.c usr.bin/usbhidctl/usbhid.c
> > > > index 921f211a280..bd0b5da0222 100644
> > > > --- usr.bin/usbhidctl/usbhid.c
> > > > +++ usr.bin/usbhidctl/usbhid.c
> > > > @@ -106,6 +106,11 @@ static struct {
> > > > #define REPORT_MAXVAL 2
> > > > };
> > > >
> > > > +struct report_desc {
> > > > + uint32_t size;
> > > > + uint8_t data[1];
> > > > +};
> > >
> > > This structure is defined in lib/libusbhid/usbvar.h which is not
> > > installed. Maybe it should and avoid this repetition and potential ABI
> > > breaks (quite unlikely but still)?
> >
> > Another approach which keeps the structure opaque is the extend the
> > usbhid(3) API with something like:
> >
> > void
> > hid_get_report_desc_data(report_desc_t d, uint8_t **data, uint32_t
> > *size)
> > {
> > *data = d->data;
> > *size = d->size;
> > }
>
> Here's a version of that:
ok anton@
Re: usbhidctl: add -R flag to dump raw report descriptor bytes
On Wed, 26 May 2021 at 08:13:52 +0200, Anton Lindqvist wrote:
> On Tue, May 25, 2021 at 08:31:14AM +0200, Anton Lindqvist wrote:
> > On Mon, May 24, 2021 at 09:17:26AM -0500, joshua stein wrote:
> > > This is useful for parsing the report descriptor with a different
> > > tool to find issues with our HID parser.
> > >
> > > I've found https://eleccelerator.com/usbdescreqparser/ to be
> > > helpful.
> > >
> > >
> > > diff --git usr.bin/usbhidctl/usbhid.c usr.bin/usbhidctl/usbhid.c
> > > index 921f211a280..bd0b5da0222 100644
> > > --- usr.bin/usbhidctl/usbhid.c
> > > +++ usr.bin/usbhidctl/usbhid.c
> > > @@ -106,6 +106,11 @@ static struct {
> > > #define REPORT_MAXVAL 2
> > > };
> > >
> > > +struct report_desc {
> > > + uint32_t size;
> > > + uint8_t data[1];
> > > +};
> >
> > This structure is defined in lib/libusbhid/usbvar.h which is not
> > installed. Maybe it should and avoid this repetition and potential ABI
> > breaks (quite unlikely but still)?
>
> Another approach which keeps the structure opaque is the extend the
> usbhid(3) API with something like:
>
> void
> hid_get_report_desc_data(report_desc_t d, uint8_t **data, uint32_t
> *size)
> {
> *data = d->data;
> *size = d->size;
> }
Here's a version of that:
diff --git lib/libusbhid/Makefile lib/libusbhid/Makefile
index 2d5a75004ad..be1adde806d 100644
--- lib/libusbhid/Makefile
+++ lib/libusbhid/Makefile
@@ -9,7 +9,7 @@ SRCS= descr.c parse.c usage.c data.c
CPPFLAGS+= -I${.CURDIR}
includes:
- @cd ${.CURDIR}; cmp -s usbhid.h ${DESTDIR}/usr/include/usbhid.h || \
+ cd ${.CURDIR}; cmp -s usbhid.h ${DESTDIR}/usr/include/usbhid.h || \
${INSTALL} ${INSTALL_COPY} -m 444 -o $(BINOWN) -g $(BINGRP) usbhid.h \
${DESTDIR}/usr/include
diff --git lib/libusbhid/descr.c lib/libusbhid/descr.c
index 9d81c2f0e3f..963108f644c 100644
--- lib/libusbhid/descr.c
+++ lib/libusbhid/descr.c
@@ -71,3 +71,10 @@ hid_dispose_report_desc(report_desc_t r)
free(r);
}
+
+void
+hid_get_report_desc_data(report_desc_t d, uint8_t **data, uint32_t *size)
+{
+ *data = d->data;
+ *size = d->size;
+}
diff --git lib/libusbhid/usbhid.3 lib/libusbhid/usbhid.3
index 62771a95798..61b2d3fe2a9 100644
--- lib/libusbhid/usbhid.3
+++ lib/libusbhid/usbhid.3
@@ -33,6 +33,7 @@
.Nm hid_get_report_desc ,
.Nm hid_use_report_desc ,
.Nm hid_dispose_report_desc ,
+.Nm hid_get_report_desc_data ,
.Nm hid_start_parse ,
.Nm hid_end_parse ,
.Nm hid_get_item ,
@@ -55,6 +56,8 @@
.Fn hid_use_report_desc "unsigned char *data" "unsigned int size"
.Ft void
.Fn hid_dispose_report_desc "report_desc_t d"
+.Ft void
+.Fn hid_get_report_desc_data "report_desc_t d" "uint8_t **data" "uint32_t
*size"
.Ft hid_data_t
.Fn hid_start_parse "report_desc_t d" "int kindset" "int id"
.Ft void
@@ -104,7 +107,8 @@ with a file descriptor obtained by opening a
device.
Alternatively a data buffer containing the report descriptor can be passed into
.Fn hid_use_report_desc .
-The data is copied into an internal structure.
+The data is copied into an internal structure which can be accessed with
+.Fn hid_get_report_desc_data .
When the report descriptor is no longer needed it should be freed by calling
.Fn hid_dispose_report_desc .
The type
diff --git lib/libusbhid/usbhid.h lib/libusbhid/usbhid.h
index 39adb4ad02f..8694c3aea1d 100644
--- lib/libusbhid/usbhid.h
+++ lib/libusbhid/usbhid.h
@@ -77,6 +77,7 @@ typedef struct hid_item {
report_desc_t hid_get_report_desc(int file);
report_desc_t hid_use_report_desc(unsigned char *data, unsigned int size);
void hid_dispose_report_desc(report_desc_t);
+void hid_get_report_desc_data(report_desc_t, uint8_t **, uint32_t *);
/* Parsing of a HID report descriptor, parse.c: */
hid_data_t hid_start_parse(report_desc_t d, int kindset, int id);
