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.
