factor out ipv4 and ipv6 initial packet sanity checks for bridges
if you're looking at an ip header, it makes sense to do some checks to
make sure that the values and addresses make some sense. the canonical
versions of these checks are in the ipv4 and ipv6 input paths, which
makes sense. when bridge(4) is about to run packets through pf it makes
sure the ip headers are sane before first, which i think also makes
sense. veb and tpmr don't do these checks before they run pf, but i
think they should. however, duplicating the code again doesn't appeal to
me.
this factors the ip checks out in the ip_input path, and uses that code
from bridge, veb, and tpmr.
this is mostly shuffling the deck chairs, but ipv6 is moved around a bit
more than ipv4, so some eyes and tests would be appreciated.
in the future i think the ipv6 code should do length checks like the
ipv4 code does too. this diff is big enough as it is though.
ok?
Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.354
diff -u -p -r1.354 if_bridge.c
--- net/if_bridge.c 5 Mar 2021 06:44:09 - 1.354
+++ net/if_bridge.c 31 May 2021 04:21:51 -
@@ -1674,61 +1674,12 @@ bridge_ip(struct ifnet *brifp, int dir,
switch (etype) {
case ETHERTYPE_IP:
- if (m->m_pkthdr.len < sizeof(struct ip))
- goto dropit;
-
- /* Copy minimal header, and drop invalids */
- if (m->m_len < sizeof(struct ip) &&
- (m = m_pullup(m, sizeof(struct ip))) == NULL) {
- ipstat_inc(ips_toosmall);
+ m = ipv4_check(ifp, m);
+ if (m == NULL)
return (NULL);
- }
- ip = mtod(m, struct ip *);
-
- if (ip->ip_v != IPVERSION) {
- ipstat_inc(ips_badvers);
- goto dropit;
- }
-
- hlen = ip->ip_hl << 2; /* get whole header length */
- if (hlen < sizeof(struct ip)) {
- ipstat_inc(ips_badhlen);
- goto dropit;
- }
-
- if (hlen > m->m_len) {
- if ((m = m_pullup(m, hlen)) == NULL) {
- ipstat_inc(ips_badhlen);
- return (NULL);
- }
- ip = mtod(m, struct ip *);
- }
-
- if ((m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_OK) == 0) {
- if (m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_BAD) {
- ipstat_inc(ips_badsum);
- goto dropit;
- }
-
- ipstat_inc(ips_inswcsum);
- if (in_cksum(m, hlen) != 0) {
- ipstat_inc(ips_badsum);
- goto dropit;
- }
- }
-
- if (ntohs(ip->ip_len) < hlen)
- goto dropit;
- if (m->m_pkthdr.len < ntohs(ip->ip_len))
- goto dropit;
- if (m->m_pkthdr.len > ntohs(ip->ip_len)) {
- if (m->m_len == m->m_pkthdr.len) {
- m->m_len = ntohs(ip->ip_len);
- m->m_pkthdr.len = ntohs(ip->ip_len);
- } else
- m_adj(m, ntohs(ip->ip_len) - m->m_pkthdr.len);
- }
+ ip = mtod(m, struct ip *);
+ hlen = ip->ip_hl << 2;
#ifdef IPSEC
if ((brifp->if_flags & IFF_LINK2) == IFF_LINK2 &&
@@ -1772,23 +1723,10 @@ bridge_ip(struct ifnet *brifp, int dir,
break;
#ifdef INET6
- case ETHERTYPE_IPV6: {
- struct ip6_hdr *ip6;
-
- if (m->m_len < sizeof(struct ip6_hdr)) {
- if ((m = m_pullup(m, sizeof(struct ip6_hdr)))
- == NULL) {
- ip6stat_inc(ip6s_toosmall);
- return (NULL);
- }
- }
-
- ip6 = mtod(m, struct ip6_hdr *);
-
- if ((ip6->ip6_vfc & IPV6_VERSION_MASK) != IPV6_VERSION) {
- ip6stat_inc(ip6s_badvers);
- goto dropit;
- }
+ case ETHERTYPE_IPV6:
+ m = ipv6_check(ifp, m);
+ if (m == NULL)
+ return (NULL);
#ifdef IPSEC
hlen = sizeof(struct ip6_hdr);
@@ -1819,7 +1757,6 @@ bridge_ip(struct ifnet *brifp, int dir,
#endif /* NPF > 0 */
break;
- }
#endif /* INET6 */
default:
Index: net/if_tpmr.c
===
RCS file: /cvs/src/sys/net/if_tpmr.c,v
retrieving revision 1.26
diff -u -p -r1.26 if_tpmr.c
---
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: ftpd(8): Convert K function definitions to modern C
On Sun, 30 May 2021 17:47:34 +0200, Jan Klemkow wrote: > Convert K function definitions to modern C. OK millert@ - todd
Re: iwm: avoid 'mac clock not ready' panic
> Date: Sun, 30 May 2021 22:26:09 +0200
> From: Stefan Sperling
>
> Steven observed a panic ("iwm0: mac clock not ready") while testing
> the iwm firmware update patch on a 9560 device.
> I've also seen this happen one time, at some point during development.
>
> In hindsight it is a bad idea to look at hardware register state here.
> The point of iwm_nic_assert_locked() is to verify that iwm_nic_lock() has
> been called somewhere up in the call stack. Checking our own lock counter
> is sufficient for this purpose.
>
> If locking the device worked then these registers had the expected state at
> that time and our lock counter was incremented. Apparently if the device runs
> into some issue later the state of these registers may change and trigger
> these panics. Instead we want to handle such failures gracefully and reset
> the device.
>
> ok?
ok kettenis@
> (For the curious: iwm_nic_lock() is supposed to avoid a situation where the
> device enters some low power state while the driver expects the device to
> quickly respond to certain I/O requests. That's all I know.)
>
> diff 385a08f3e862586df8f1803dfa09fc765a5c3610 /usr/src
> blob - 4b502468fea796f103fb84237146879e8c4df267
> file + sys/dev/pci/if_iwm.c
> --- sys/dev/pci/if_iwm.c
> +++ sys/dev/pci/if_iwm.c
> @@ -1069,11 +1069,6 @@ iwm_nic_lock(struct iwm_softc *sc)
> void
> iwm_nic_assert_locked(struct iwm_softc *sc)
> {
> - uint32_t reg = IWM_READ(sc, IWM_CSR_GP_CNTRL);
> - if ((reg & IWM_CSR_GP_CNTRL_REG_FLAG_MAC_CLOCK_READY) == 0)
> - panic("%s: mac clock not ready", DEVNAME(sc));
> - if (reg & IWM_CSR_GP_CNTRL_REG_FLAG_GOING_TO_SLEEP)
> - panic("%s: mac gone to sleep", DEVNAME(sc));
> if (sc->sc_nic_locks <= 0)
> panic("%s: nic locks counter %d", DEVNAME(sc),
> sc->sc_nic_locks);
> }
>
>
iwm: avoid 'mac clock not ready' panic
Steven observed a panic ("iwm0: mac clock not ready") while testing
the iwm firmware update patch on a 9560 device.
I've also seen this happen one time, at some point during development.
In hindsight it is a bad idea to look at hardware register state here.
The point of iwm_nic_assert_locked() is to verify that iwm_nic_lock() has
been called somewhere up in the call stack. Checking our own lock counter
is sufficient for this purpose.
If locking the device worked then these registers had the expected state at
that time and our lock counter was incremented. Apparently if the device runs
into some issue later the state of these registers may change and trigger
these panics. Instead we want to handle such failures gracefully and reset
the device.
ok?
(For the curious: iwm_nic_lock() is supposed to avoid a situation where the
device enters some low power state while the driver expects the device to
quickly respond to certain I/O requests. That's all I know.)
diff 385a08f3e862586df8f1803dfa09fc765a5c3610 /usr/src
blob - 4b502468fea796f103fb84237146879e8c4df267
file + sys/dev/pci/if_iwm.c
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -1069,11 +1069,6 @@ iwm_nic_lock(struct iwm_softc *sc)
void
iwm_nic_assert_locked(struct iwm_softc *sc)
{
- uint32_t reg = IWM_READ(sc, IWM_CSR_GP_CNTRL);
- if ((reg & IWM_CSR_GP_CNTRL_REG_FLAG_MAC_CLOCK_READY) == 0)
- panic("%s: mac clock not ready", DEVNAME(sc));
- if (reg & IWM_CSR_GP_CNTRL_REG_FLAG_GOING_TO_SLEEP)
- panic("%s: mac gone to sleep", DEVNAME(sc));
if (sc->sc_nic_locks <= 0)
panic("%s: nic locks counter %d", DEVNAME(sc),
sc->sc_nic_locks);
}
ftpd(8): Convert K function definitions to modern C
Hi,
Convert K function definitions to modern C.
OK?
bye,
Jan
Index: ftpcmd.y
===
RCS file: /cvs/src/libexec/ftpd/ftpcmd.y,v
retrieving revision 1.72
diff -u -p -r1.72 ftpcmd.y
--- ftpcmd.y23 May 2021 17:01:21 - 1.72
+++ ftpcmd.y30 May 2021 15:32:50 -
@@ -1072,9 +1072,7 @@ static int yylex(void);
extern int epsvall;
static struct tab *
-lookup(p, cmd)
- struct tab *p;
- const char *cmd;
+lookup(struct tab *p, const char *cmd)
{
for (; p->name != NULL; p++)
@@ -1089,9 +1087,7 @@ lookup(p, cmd)
* get_line - a hacked up version of fgets to ignore TELNET escape codes.
*/
int
-get_line(s, n)
- char *s;
- int n;
+get_line(char *s, int n)
{
int c;
char *cs;
@@ -1176,8 +1172,7 @@ get_line(s, n)
/*ARGSUSED*/
void
-toolong(signo)
- int signo;
+toolong(int signo)
{
struct syslog_data sdata = SYSLOG_DATA_INIT;
@@ -1190,7 +1185,7 @@ toolong(signo)
}
static int
-yylex()
+yylex(void)
{
static int cpos;
char *cp, *cp2;
@@ -1429,8 +1424,7 @@ yylex()
}
void
-upper(s)
- char *s;
+upper(char *s)
{
char *p;
@@ -1439,9 +1433,7 @@ upper(s)
}
static void
-help(ctab, s)
- struct tab *ctab;
- char *s;
+help(struct tab *ctab, char *s)
{
struct tab *c;
int width, NCMDS;
@@ -1504,8 +1496,7 @@ help(ctab, s)
}
static void
-sizecmd(filename)
- const char *filename;
+sizecmd(const char *filename)
{
switch (type) {
case TYPE_L:
Index: monitor.c
===
RCS file: /cvs/src/libexec/ftpd/monitor.c,v
retrieving revision 1.28
diff -u -p -r1.28 monitor.c
--- monitor.c 20 May 2021 15:21:03 - 1.28
+++ monitor.c 30 May 2021 15:38:52 -
@@ -206,7 +206,7 @@ monitor_init(void)
* for the user-privileged slave process and 1 for the monitor process.
*/
int
-monitor_post_auth()
+monitor_post_auth(void)
{
slave_pid = fork();
if (slave_pid == -1)
couple devices found on a intel nuc 10i7FNH
ahoi, recognize couple devices found in a Intel Nuc 10i7FNH. Since the JHL7540 exists with a few pci ids, this suffixes the existing one so it matches with the new addition. felix Index: sys/dev/pci/pcidevs === RCS file: /cvs/src/sys/dev/pci/pcidevs,v retrieving revision 1.1970 diff -u -p -u -r1.1970 pcidevs --- sys/dev/pci/pcidevs 19 May 2021 05:20:48 - 1.1970 +++ sys/dev/pci/pcidevs 30 May 2021 19:24:05 - @@ -302,6 +302,7 @@ vendor ALTIMA 0x173b Altima vendor ANTARES 0x1754 Antares Microsystems vendor CAVIUM 0x177d Cavium vendor BELKIN2 0x1799 Belkin +vendor GENESYS 0x17a0 Genesys Logic vendor LENOVO 0x17aa Lenovo vendor HAWKING 0x17b3 Hawking Technology vendor NETCHIP 0x17cc NetChip Technology @@ -3034,6 +3035,9 @@ product FUSIONIO IOXTREME_PRO 0x1007 ioX /* Future Domain products */ product FUTUREDOMAIN TMC_18C30 0x TMC-18C30 +/* Genesys Logic products */ +product GENESYS GLI_9755 0x9755 SDHC + /* Guillemot products */ product GEMTEK PR103 0x1001 PR103 @@ -3977,9 +3981,12 @@ product INTEL I219_V90x15e2 I219-V product INTEL I219_LM5 0x15e3 I219-LM product INTEL X550EM_A_1G_T0x15e4 X553 SGMII product INTEL X550EM_A_1G_T_L 0x15e5 X553 SGMII -product INTEL JHL7540_PCIE 0x15ea JHL7540 Thunderbolt 3 -product INTEL JHL7540 0x15eb JHL7540 Thunderbolt 3 -product INTEL JHL7540_XHCI 0x15ec JHL7540 Thunderbolt 3 +product INTEL JHL7540_PCIE_1 0x15e7 JHL7540 Thunderbolt 3 +product INTEL JHL7540_10x15e8 JHL7540 Thunderbolt 3 +product INTEL JHL7540_XHCI_1 0x15e9 JHL7540 Thunderbolt 3 +product INTEL JHL7540_PCIE_2 0x15ea JHL7540 Thunderbolt 3 +product INTEL JHL7540_20x15eb JHL7540 Thunderbolt 3 +product INTEL JHL7540_XHCI_2 0x15ec JHL7540 Thunderbolt 3 product INTEL I225_LM 0x15f2 I225-LM product INTEL I225_V 0x15f3 I225-V product INTEL I219_LM150x15f4 I219-LM -- GPG/PGP: 7A0B612C / 5F4D 9B06 C240 3250 35BF 66ED 1AD3 A9B8 7A0B 612C
Re: Avoid shifting of a negative value in sys_adjfreq()
Visa Hankala: > However, wouldn't it be better if the code avoided the > situation, for example by defining ADJFREQ_MIN as the negative > of ADJFREQ_MAX? Indeed it would. ok naddy@ > --- kern/kern_time.c 23 Dec 2020 20:45:02 - 1.151 > +++ kern/kern_time.c 30 May 2021 15:38:09 - > @@ -396,7 +396,7 @@ sys_settimeofday(struct proc *p, void *v > } > > #define ADJFREQ_MAX (5LL << 32) > -#define ADJFREQ_MIN (-5LL << 32) > +#define ADJFREQ_MIN (-ADJFREQ_MAX) > > int > sys_adjfreq(struct proc *p, void *v, register_t *retval) > -- Christian "naddy" Weisgerber na...@mips.inka.de
Avoid shifting of a negative value in sys_adjfreq()
When cross-compiling the kernel using ports clang (tsk tsk), the compiler reports the following: src/sys/kern/kern_time.c:418:11: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value] if (f < ADJFREQ_MIN || f > ADJFREQ_MAX) ^ src/sys/kern/kern_time.c:399:35: note: expanded from macro 'ADJFREQ_MIN' #define ADJFREQ_MIN (-5LL << 32) ^ Normally this does not occur because of the implicit -fwrapv option with base clang. However, wouldn't it be better if the code avoided the situation, for example by defining ADJFREQ_MIN as the negative of ADJFREQ_MAX? Index: kern/kern_time.c === RCS file: src/sys/kern/kern_time.c,v retrieving revision 1.151 diff -u -p -r1.151 kern_time.c --- kern/kern_time.c23 Dec 2020 20:45:02 - 1.151 +++ kern/kern_time.c30 May 2021 15:38:09 - @@ -396,7 +396,7 @@ sys_settimeofday(struct proc *p, void *v } #define ADJFREQ_MAX (5LL << 32) -#define ADJFREQ_MIN (-5LL << 32) +#define ADJFREQ_MIN (-ADJFREQ_MAX) int sys_adjfreq(struct proc *p, void *v, register_t *retval)
Re: uvideo(4) new quirk flag UVIDEO_FLAG_NOATTACH
On Mon, 5 Apr 2021 23:54:31 +0200
Marcus Glocker wrote:
> On Mon, Apr 05, 2021 at 11:27:10PM +0200, Mark Kettenis wrote:
>
> [...]
>
> > > > > How common is it to explain the system behavior in cases like
> > > > > this? Would it be less surprising (generate less misc@
> > > > > traffic) if we printed a note explaining why the camera was
> > > > > skipped?
> > > >
> > > > I wouldn't print a specific message per unsupported device, but
> > > > I think a generic message that the video device isn't supported
> > > > would make sense. Something like that maybe? Obviously this
> > > > can print more than once, but I'm not sure if it's worth to
> > > > make that a unique print.
> > > >
> > > > E.g.:
> > > >
> > > > vmm0 at mainbus0: VMX/EPT
> > > > uvideo: device 13d3:56b2 isn't supported
> > > > uvideo: device 13d3:56b2 isn't supported
> > > > ugen0 at uhub0 port 8 "SunplusIT Inc Integrated Camera" rev
> > > > 2.01/17.11 addr 2
> > >
> > > No; match functions shouldn't print stuff.
> >
> > If you really wanted to print a message, you'll need to let
> > uvideo(4) attach and then print a "not supported" message early on
> > in the attach function and return.
>
> Right. Despite the print line, maybe this way is anyway better than
> doing this check in the match function.
While cleaning up my diffs I just found that this one hasn't been
committed. OK to get this in?
Index: sys/dev/usb/uvideo.c
===
RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
retrieving revision 1.212
diff -u -p -u -p -r1.212 uvideo.c
--- sys/dev/usb/uvideo.c5 Apr 2021 20:45:49 - 1.212
+++ sys/dev/usb/uvideo.c5 Apr 2021 21:51:11 -
@@ -490,9 +490,6 @@ uvideo_match(struct device *parent, void
/* quirk devices */
quirk = uvideo_lookup(uaa->vendor, uaa->product);
if (quirk != NULL) {
- if (quirk->flags & UVIDEO_FLAG_NOATTACH)
- return (UMATCH_NONE);
-
if (quirk->flags & UVIDEO_FLAG_REATTACH)
return (UMATCH_VENDOR_PRODUCT_CONF_IFACE);
@@ -577,6 +574,11 @@ uvideo_attach(struct device *parent, str
/* maybe the device has quirks */
sc->sc_quirk = uvideo_lookup(uaa->vendor, uaa->product);
+
+ if (sc->sc_quirk && sc->sc_quirk->flags & UVIDEO_FLAG_NOATTACH) {
+ printf("%s: device not supported\n", DEVNAME(sc));
+ return;
+ }
if (sc->sc_quirk && sc->sc_quirk->ucode_name)
config_mountroot(self, uvideo_attach_hook);
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@
