On Thu, Apr 14, 2022 at 04:28:37PM +0200, Alexander Bluhm wrote: > Hi, > > When kbd -l is executed as regular user, it fails silently. > > $ kbd -l > $ echo $? > 0 > > Error handling is a bit tricky. We want the first error if no > device is available. > > $ ./kbd -l > kbd: open /dev/wskbd[0-9]: Permission denied > $ echo $? > 1 > > ok? > > bluhm > > Index: sbin/kbd/kbd_wscons.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sbin/kbd/kbd_wscons.c,v > retrieving revision 1.34 > diff -u -p -r1.34 kbd_wscons.c > --- sbin/kbd/kbd_wscons.c 22 Jan 2020 06:24:07 -0000 1.34 > +++ sbin/kbd/kbd_wscons.c 14 Apr 2022 14:21:17 -0000 > @@ -150,7 +150,7 @@ kbd_list(void) > { > int kbds[SA_MAX]; > struct wskbd_encoding_data encs[SA_MAX]; > - int fd, i, kbtype, t; > + int fd, i, kbtype, t, error = 0; > char device[PATH_MAX]; > > memset(kbds, 0, sizeof(kbds)); > @@ -162,7 +162,14 @@ kbd_list(void) > fd = open(device, O_WRONLY); > if (fd == -1) > fd = open(device, O_RDONLY); > - if (fd >= 0) { > + if (fd == -1) { > + /* remember the first error number */ > + if (error == 0) > + error = errno; > + } else { > + /* at least one success, do not print error */ > + error = -1; > + > if (ioctl(fd, WSKBDIO_GTYPE, &kbtype) == -1) > err(1, "WSKBDIO_GTYPE"); > switch (kbtype) { > @@ -207,6 +214,10 @@ kbd_list(void) > } > close(fd); > } > + } > + if (error > 0) { > + errno = error; > + err(1, "open /dev/wskbd[0-9]"); > } > > for (i = 0; i < SA_MAX; i++) > > I was annoyed by this as well, so I welcome better code. It looks 99% okay for me.
I'm not quite fond of the error reports, though... they could be more specific about which device is doing what. - we keep track of the first error, so it should probably talk about /dev/wskbd0 directly ? - by comparison, the message for the WSKBDIO_GTYPE doesn't mention the device name. I think err(1, "WKBDIO_GTYPE on %s", device) might be slightly more helpful.