Re: usbhidctl: add -R flag to dump raw report descriptor bytes

[joshua stein]

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);