Re: Fix kbd -l

[Mark Kettenis]

> From: Jeremie Courreges-Anglas 
> Date: Fri, 30 Sep 2016 17:00:45 +0200
> 
> Mark Kettenis  writes:
> 
> [...]
> 
> >> > -for (i = 0; kbdenc_tab[i].value; i++)
> >> > -printf("%s\n", kbdenc_tab[i].name);
> >> > +for (i = 0; i < encs->nencodings; i++) {
> >> > +n = _tab[0];
> >> > +found = 0;
> >> > +encoding = encs->encodings[i];
> >> > +while (n->value) {
> >> > +if (n->value == KB_ENCODING(encoding)) {
> >> > +printf("%s", n->name);
> >> > +found++;
> >> > +}
> >> > +n++;
> >> > +}
> >> 
> >> This looks a lot like a for loop.
> >
> > Right.  But there are some similar loops that are already written this
> > way.  So I left it the way I wrote^H^H^H^H^Hcopied it.  Feel free to
> > submite a separate patch that fixes this.
> 
> Ah, I see.  Here's the diff.

Thanks!  ok kettenis@

> Index: kbd_wscons.c
> ===
> RCS file: /cvs/src/sbin/kbd/kbd_wscons.c,v
> retrieving revision 1.31
> diff -u -p -p -u -r1.31 kbd_wscons.c
> --- kbd_wscons.c  30 Sep 2016 12:07:23 -  1.31
> +++ kbd_wscons.c  30 Sep 2016 14:55:03 -
> @@ -100,27 +100,23 @@ kbd_show_enc(struct wskbd_encoding_data 
>   kbtype_tab[idx]);
>  
>   for (i = 0; i < encs->nencodings; i++) {
> - n = _tab[0];
>   found = 0;
>   encoding = encs->encodings[i];
> - while (n->value) {
> + for (n = _tab[0]; n->value; n++) {
>   if (n->value == KB_ENCODING(encoding)) {
>   printf("%s", n->name);
>   found++;
>   }
> - n++;
>   }
>   if (found == 0)
>   printf("", KB_ENCODING(encoding));
> - n = _tab[0];
>   found = 0;
>   variant = KB_VARIANT(encoding);
> - while (n->value) {
> + for (n = _tab[0]; n->value; n++) {
>   if ((n->value & KB_VARIANT(encoding)) == n->value) {
>   printf(".%s", n->name);
>   variant &= ~n->value;
>   }
> - n++;
>   }
>   if (variant != 0)
>   printf(".", variant);
> @@ -248,11 +244,9 @@ kbd_set(char *name, int verbose)
>   *b++ = *c++;
>   *b = '\0';
>   v = 0;
> - n = _tab[0];
> - while (n->value) {
> + for (n = _tab[0]; n->value; n++) {
>   if (strcmp(n->name, buf) == 0)
>   v = n->value;
> - n++;
>   }
>   if (v == 0)
>   errx(1, "unknown variant %s", buf);
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 

Re: Fix kbd -l

[Jeremie Courreges-Anglas]

Mark Kettenis  writes:

[...]

>> > -  for (i = 0; kbdenc_tab[i].value; i++)
>> > -  printf("%s\n", kbdenc_tab[i].name);
>> > +  for (i = 0; i < encs->nencodings; i++) {
>> > +  n = _tab[0];
>> > +  found = 0;
>> > +  encoding = encs->encodings[i];
>> > +  while (n->value) {
>> > +  if (n->value == KB_ENCODING(encoding)) {
>> > +  printf("%s", n->name);
>> > +  found++;
>> > +  }
>> > +  n++;
>> > +  }
>> 
>> This looks a lot like a for loop.
>
> Right.  But there are some similar loops that are already written this
> way.  So I left it the way I wrote^H^H^H^H^Hcopied it.  Feel free to
> submite a separate patch that fixes this.

Ah, I see.  Here's the diff.


Index: kbd_wscons.c
===
RCS file: /cvs/src/sbin/kbd/kbd_wscons.c,v
retrieving revision 1.31
diff -u -p -p -u -r1.31 kbd_wscons.c
--- kbd_wscons.c30 Sep 2016 12:07:23 -  1.31
+++ kbd_wscons.c30 Sep 2016 14:55:03 -
@@ -100,27 +100,23 @@ kbd_show_enc(struct wskbd_encoding_data 
kbtype_tab[idx]);
 
for (i = 0; i < encs->nencodings; i++) {
-   n = _tab[0];
found = 0;
encoding = encs->encodings[i];
-   while (n->value) {
+   for (n = _tab[0]; n->value; n++) {
if (n->value == KB_ENCODING(encoding)) {
printf("%s", n->name);
found++;
}
-   n++;
}
if (found == 0)
printf("", KB_ENCODING(encoding));
-   n = _tab[0];
found = 0;
variant = KB_VARIANT(encoding);
-   while (n->value) {
+   for (n = _tab[0]; n->value; n++) {
if ((n->value & KB_VARIANT(encoding)) == n->value) {
printf(".%s", n->name);
variant &= ~n->value;
}
-   n++;
}
if (variant != 0)
printf(".", variant);
@@ -248,11 +244,9 @@ kbd_set(char *name, int verbose)
*b++ = *c++;
*b = '\0';
v = 0;
-   n = _tab[0];
-   while (n->value) {
+   for (n = _tab[0]; n->value; n++) {
if (strcmp(n->name, buf) == 0)
v = n->value;
-   n++;
}
if (v == 0)
errx(1, "unknown variant %s", buf);

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: Fix kbd -l

[Mark Kettenis]

> From: Jeremie Courreges-Anglas 
> Date: Fri, 30 Sep 2016 13:32:57 +0200
> 
> Mark Kettenis  writes:
> 
> >> Date: Thu, 29 Sep 2016 14:19:43 +0200 (CEST)
> >> From: Mark Kettenis 
> >> 
> >> This diff adds a WSKBDIO_GETENCODINGS ioctl and uses it to print a
> >> list of supported encodings like the old kvm groveling code did.  The
> >> ioctl will clamp the number of entries that are returns to the number
> >> that was passed in.  This means that if the number returned is the
> >> same as the number passed in, there might be more entries and you
> >> probably want to retry with a larger buffer.  The new implementation
> >> does this.
> >
> > Thanks to tb@ for testing this on a ramdisk.  Here is a slightly
> > better diff that frees the allocated memory.
> >
> > ok?
> 
> Looks fine to me, ok jca@

Thanks,

> Nitpicking below,

I selectively applied those; see comments below.

> > Index: sbin/kbd/kbd_wscons.c
> > ===
> > RCS file: /cvs/src/sbin/kbd/kbd_wscons.c,v
> > retrieving revision 1.30
> > diff -u -p -r1.30 kbd_wscons.c
> > --- sbin/kbd/kbd_wscons.c   27 Sep 2016 22:03:49 -  1.30
> > +++ sbin/kbd/kbd_wscons.c   29 Sep 2016 18:12:48 -
> > @@ -34,6 +34,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -82,31 +83,83 @@ struct nameint kbdvar_tab[] = {
> >  
> >  extern char *__progname;
> >  
> > -void   kbd_show_enc(int idx);
> > +void   kbd_show_enc(struct wskbd_encoding_data *encs, int idx);
> > +void   kbd_get_encs(int fd, struct wskbd_encoding_data *encs);
> >  void   kbd_list(void);
> >  void   kbd_set(char *name, int verbose);
> >  
> >  void
> > -kbd_show_enc(int idx)
> > +kbd_show_enc(struct wskbd_encoding_data *encs, int idx)
> >  {
> > +   int found;
> > +   kbd_t encoding, variant;
> > +   struct nameint *n;
> > int i;
> >  
> > printf("tables available for %s keyboard:\nencoding\n\n",
> > kbtype_tab[idx]);
> >  
> > -   for (i = 0; kbdenc_tab[i].value; i++)
> > -   printf("%s\n", kbdenc_tab[i].name);
> > +   for (i = 0; i < encs->nencodings; i++) {
> > +   n = _tab[0];
> > +   found = 0;
> > +   encoding = encs->encodings[i];
> > +   while (n->value) {
> > +   if (n->value == KB_ENCODING(encoding)) {
> > +   printf("%s", n->name);
> > +   found++;
> > +   }
> > +   n++;
> > +   }
> 
> This looks a lot like a for loop.

Right.  But there are some similar loops that are already written this
way.  So I left it the way I wrote^H^H^H^H^Hcopied it.  Feel free to
submite a separate patch that fixes this.

Re: Fix kbd -l

[Jeremie Courreges-Anglas]

Mark Kettenis  writes:

>> Date: Thu, 29 Sep 2016 14:19:43 +0200 (CEST)
>> From: Mark Kettenis 
>> 
>> This diff adds a WSKBDIO_GETENCODINGS ioctl and uses it to print a
>> list of supported encodings like the old kvm groveling code did.  The
>> ioctl will clamp the number of entries that are returns to the number
>> that was passed in.  This means that if the number returned is the
>> same as the number passed in, there might be more entries and you
>> probably want to retry with a larger buffer.  The new implementation
>> does this.
>
> Thanks to tb@ for testing this on a ramdisk.  Here is a slightly
> better diff that frees the allocated memory.
>
> ok?

Looks fine to me, ok jca@

Nitpicking below,

>
> Index: sys/dev/wscons/wsconsio.h
> ===
> RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
> retrieving revision 1.75
> diff -u -p -r1.75 wsconsio.h
> --- sys/dev/wscons/wsconsio.h 14 Sep 2016 03:25:51 -  1.75
> +++ sys/dev/wscons/wsconsio.h 29 Sep 2016 18:12:48 -
> @@ -207,6 +207,12 @@ struct wskbd_backlight {
>  #define  WSKBD_TRANSLATED0
>  #define  WSKBD_RAW   1
>  
> +struct wskbd_encoding_data {
> + int nencodings;
> + kbd_t   *encodings;
> +};
> +#define WSKBDIO_GETENCODINGS _IOWR('W', 21, struct wskbd_encoding_data)
> +
>  /*
>   * Mouse ioctls (32 - 63)
>   */
> Index: sys/dev/wscons/wskbd.c
> ===
> RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 wskbd.c
> --- sys/dev/wscons/wskbd.c12 Dec 2015 12:30:18 -  1.83
> +++ sys/dev/wscons/wskbd.c29 Sep 2016 18:12:48 -
> @@ -1001,9 +1001,11 @@ wskbd_displayioctl(struct device *dev, u
>   struct wskbd_bell_data *ubdp, *kbdp;
>   struct wskbd_keyrepeat_data *ukdp, *kkdp;
>   struct wskbd_map_data *umdp;
> + struct wskbd_encoding_data *uedp;
>   kbd_t enc;
>   void *buf;
>   int len, error;
> + int count, i;

Maybe merge those two lines?

>  
>   switch (cmd) {
>   case WSKBDIO_BELL:
> @@ -1159,6 +1161,20 @@ getkeyrepeat:
>   wsmux_set_layout(sc->sc_base.me_parent, enc);
>  #endif
>   return (0);
> +
> + case WSKBDIO_GETENCODINGS:
> + uedp = (struct wskbd_encoding_data *)data;
> + for (count = 0; sc->id->t_keymap.keydesc[count].name; count++)
> + ;
> + if (uedp->nencodings > count)
> + uedp->nencodings = count;
> + for (i = 0; i < uedp->nencodings; i++) {
> + error = copyout(>id->t_keymap.keydesc[i].name,
> + >encodings[i], sizeof(kbd_t));
> + if (error)
> + return (error);
> + }
> + return(0);
+space^

>  
>   case WSKBDIO_GETBACKLIGHT:
>   if (wskbd_get_backlight != NULL)
> Index: sbin/kbd/kbd_wscons.c
> ===
> RCS file: /cvs/src/sbin/kbd/kbd_wscons.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 kbd_wscons.c
> --- sbin/kbd/kbd_wscons.c 27 Sep 2016 22:03:49 -  1.30
> +++ sbin/kbd/kbd_wscons.c 29 Sep 2016 18:12:48 -
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -82,31 +83,83 @@ struct nameint kbdvar_tab[] = {
>  
>  extern char *__progname;
>  
> -void kbd_show_enc(int idx);
> +void kbd_show_enc(struct wskbd_encoding_data *encs, int idx);
> +void kbd_get_encs(int fd, struct wskbd_encoding_data *encs);
>  void kbd_list(void);
>  void kbd_set(char *name, int verbose);
>  
>  void
> -kbd_show_enc(int idx)
> +kbd_show_enc(struct wskbd_encoding_data *encs, int idx)
>  {
> + int found;
> + kbd_t encoding, variant;
> + struct nameint *n;
>   int i;
>  
>   printf("tables available for %s keyboard:\nencoding\n\n",
>   kbtype_tab[idx]);
>  
> - for (i = 0; kbdenc_tab[i].value; i++)
> - printf("%s\n", kbdenc_tab[i].name);
> + for (i = 0; i < encs->nencodings; i++) {
> + n = _tab[0];
> + found = 0;
> + encoding = encs->encodings[i];
> + while (n->value) {
> + if (n->value == KB_ENCODING(encoding)) {
> + printf("%s", n->name);
> + found++;
> + }
> + n++;
> + }

This looks a lot like a for loop.

> + if (found == 0)
> + printf("", KB_ENCODING(encoding));
> + n = _tab[0];
> + found = 0;
> + variant = KB_VARIANT(encoding);
> + while (n->value) {
> + if ((n->value & KB_VARIANT(encoding)) == n->value) {

Re: Fix kbd -l

[Mark Kettenis]

> Date: Thu, 29 Sep 2016 14:19:43 +0200 (CEST)
> From: Mark Kettenis 
> 
> This diff adds a WSKBDIO_GETENCODINGS ioctl and uses it to print a
> list of supported encodings like the old kvm groveling code did.  The
> ioctl will clamp the number of entries that are returns to the number
> that was passed in.  This means that if the number returned is the
> same as the number passed in, there might be more entries and you
> probably want to retry with a larger buffer.  The new implementation
> does this.

Thanks to tb@ for testing this on a ramdisk.  Here is a slightly
better diff that frees the allocated memory.

ok?


Index: sys/dev/wscons/wsconsio.h
===
RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.75
diff -u -p -r1.75 wsconsio.h
--- sys/dev/wscons/wsconsio.h   14 Sep 2016 03:25:51 -  1.75
+++ sys/dev/wscons/wsconsio.h   29 Sep 2016 18:12:48 -
@@ -207,6 +207,12 @@ struct wskbd_backlight {
 #defineWSKBD_TRANSLATED0
 #defineWSKBD_RAW   1
 
+struct wskbd_encoding_data {
+   int nencodings;
+   kbd_t   *encodings;
+};
+#define WSKBDIO_GETENCODINGS   _IOWR('W', 21, struct wskbd_encoding_data)
+
 /*
  * Mouse ioctls (32 - 63)
  */
Index: sys/dev/wscons/wskbd.c
===
RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
retrieving revision 1.83
diff -u -p -r1.83 wskbd.c
--- sys/dev/wscons/wskbd.c  12 Dec 2015 12:30:18 -  1.83
+++ sys/dev/wscons/wskbd.c  29 Sep 2016 18:12:48 -
@@ -1001,9 +1001,11 @@ wskbd_displayioctl(struct device *dev, u
struct wskbd_bell_data *ubdp, *kbdp;
struct wskbd_keyrepeat_data *ukdp, *kkdp;
struct wskbd_map_data *umdp;
+   struct wskbd_encoding_data *uedp;
kbd_t enc;
void *buf;
int len, error;
+   int count, i;
 
switch (cmd) {
case WSKBDIO_BELL:
@@ -1159,6 +1161,20 @@ getkeyrepeat:
wsmux_set_layout(sc->sc_base.me_parent, enc);
 #endif
return (0);
+
+   case WSKBDIO_GETENCODINGS:
+   uedp = (struct wskbd_encoding_data *)data;
+   for (count = 0; sc->id->t_keymap.keydesc[count].name; count++)
+   ;
+   if (uedp->nencodings > count)
+   uedp->nencodings = count;
+   for (i = 0; i < uedp->nencodings; i++) {
+   error = copyout(>id->t_keymap.keydesc[i].name,
+   >encodings[i], sizeof(kbd_t));
+   if (error)
+   return (error);
+   }
+   return(0);
 
case WSKBDIO_GETBACKLIGHT:
if (wskbd_get_backlight != NULL)
Index: sbin/kbd/kbd_wscons.c
===
RCS file: /cvs/src/sbin/kbd/kbd_wscons.c,v
retrieving revision 1.30
diff -u -p -r1.30 kbd_wscons.c
--- sbin/kbd/kbd_wscons.c   27 Sep 2016 22:03:49 -  1.30
+++ sbin/kbd/kbd_wscons.c   29 Sep 2016 18:12:48 -
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -82,31 +83,83 @@ struct nameint kbdvar_tab[] = {
 
 extern char *__progname;
 
-void   kbd_show_enc(int idx);
+void   kbd_show_enc(struct wskbd_encoding_data *encs, int idx);
+void   kbd_get_encs(int fd, struct wskbd_encoding_data *encs);
 void   kbd_list(void);
 void   kbd_set(char *name, int verbose);
 
 void
-kbd_show_enc(int idx)
+kbd_show_enc(struct wskbd_encoding_data *encs, int idx)
 {
+   int found;
+   kbd_t encoding, variant;
+   struct nameint *n;
int i;
 
printf("tables available for %s keyboard:\nencoding\n\n",
kbtype_tab[idx]);
 
-   for (i = 0; kbdenc_tab[i].value; i++)
-   printf("%s\n", kbdenc_tab[i].name);
+   for (i = 0; i < encs->nencodings; i++) {
+   n = _tab[0];
+   found = 0;
+   encoding = encs->encodings[i];
+   while (n->value) {
+   if (n->value == KB_ENCODING(encoding)) {
+   printf("%s", n->name);
+   found++;
+   }
+   n++;
+   }
+   if (found == 0)
+   printf("", KB_ENCODING(encoding));
+   n = _tab[0];
+   found = 0;
+   variant = KB_VARIANT(encoding);
+   while (n->value) {
+   if ((n->value & KB_VARIANT(encoding)) == n->value) {
+   printf(".%s", n->name);
+   variant &= ~n->value;
+   }
+   n++;
+   }
+   if (variant != 0)
+   printf(".", variant);
+   printf("\n");
+   }

Re: Fix kbd -l

[Theo Buehler]

On Thu, Sep 29, 2016 at 04:53:12PM +0200, Mark Kettenis wrote:
> > Date: Thu, 29 Sep 2016 16:09:09 +0200
> > From: Theo Buehler 
> > 
> > On Thu, Sep 29, 2016 at 02:19:43PM +0200, Mark Kettenis wrote:
> > > This diff adds a WSKBDIO_GETENCODINGS ioctl and uses it to print a
> > > list of supported encodings like the old kvm groveling code did.  The
> > > ioctl will clamp the number of entries that are returns to the number
> > > that was passed in.  This means that if the number returned is the
> > > same as the number passed in, there might be more entries and you
> > > probably want to retry with a larger buffer.  The new implementation
> > > does this.
> > > 
> > > Waring; this did not go through a make release yet.  And that might be
> > > relevant, since this code gets included in the ramdisks.
> > > 
> > > Thoughts?
> > > 
> > 
> > amd64 release builds fine with this patch. I tested it on both a normal
> > system and with bsd.rd, works fine.  I like the fact that extended
> > keyboard encodings are now also listed in bsd.rd.
> > 
> > There is one change of behavior: previously, if a non-root user ran
> > kbd -l, it would exit with status 1 and print the error message:
> > 
> > kbd: kvm_openfiles: /dev/mem: permission denied
> > 
> > which gave at least some indication that it must be run as root.
> > 
> > Now it fails silently with exit code 0 without printing anything since
> > opening the /dev/wskbd?  devices fails, as they are owned by root:wheel
> > with permissions 600.
> 
> If you log in on the console, it actually works for non-root users as
> well, at least for the primary keyboard.  This is because login(1)
> modifies the permissions according to /etc/fbtab.  Now if you have
> both pckbd(4) and ukbd(4), it will probably only show the encodings
> for pckbd(4), unless you change /etc/fbtab to also change the
> permissions on /dev/wskbd1.  But at that point we're quickly entering
> the realm where kbd(8) simply isn't the appropriate tools to manage
> things.
> 
> Now if you log in through xdm, you will in all likelyhood get an empty
> list.  But X has its own idea about keyboard mappings anyway.  It only
> initializes its settings based on the wscons settings.  But it never
> changes the wscons settings if you manipulate the X mappings.

Thanks for this explanation.  Indeed, I only tested the old version as
non-root on console.  Sorry about that.  Then this all makes sense.

I can't comment on your choice of design, but it does read well as it is.
So, fwiw, ok from me.

Re: Fix kbd -l

[Chris Bennett]

On Thu, Sep 29, 2016 at 04:53:12PM +0200, Mark Kettenis wrote:
> Now if you log in through xdm, you will in all likelyhood get an empty
> list.  But X has its own idea about keyboard mappings anyway.  It only
> initializes its settings based on the wscons settings.  But it never
> changes the wscons settings if you manipulate the X mappings.
> 

Yes, you can ignore my comments since it involved working under X, not
the console.

Chris Bennett

Re: Fix kbd -l

[Mark Kettenis]

> Date: Thu, 29 Sep 2016 16:09:09 +0200
> From: Theo Buehler 
> 
> On Thu, Sep 29, 2016 at 02:19:43PM +0200, Mark Kettenis wrote:
> > This diff adds a WSKBDIO_GETENCODINGS ioctl and uses it to print a
> > list of supported encodings like the old kvm groveling code did.  The
> > ioctl will clamp the number of entries that are returns to the number
> > that was passed in.  This means that if the number returned is the
> > same as the number passed in, there might be more entries and you
> > probably want to retry with a larger buffer.  The new implementation
> > does this.
> > 
> > Waring; this did not go through a make release yet.  And that might be
> > relevant, since this code gets included in the ramdisks.
> > 
> > Thoughts?
> > 
> 
> amd64 release builds fine with this patch. I tested it on both a normal
> system and with bsd.rd, works fine.  I like the fact that extended
> keyboard encodings are now also listed in bsd.rd.
> 
> There is one change of behavior: previously, if a non-root user ran
> kbd -l, it would exit with status 1 and print the error message:
> 
> kbd: kvm_openfiles: /dev/mem: permission denied
> 
> which gave at least some indication that it must be run as root.
> 
> Now it fails silently with exit code 0 without printing anything since
> opening the /dev/wskbd?  devices fails, as they are owned by root:wheel
> with permissions 600.

If you log in on the console, it actually works for non-root users as
well, at least for the primary keyboard.  This is because login(1)
modifies the permissions according to /etc/fbtab.  Now if you have
both pckbd(4) and ukbd(4), it will probably only show the encodings
for pckbd(4), unless you change /etc/fbtab to also change the
permissions on /dev/wskbd1.  But at that point we're quickly entering
the realm where kbd(8) simply isn't the appropriate tools to manage
things.

Now if you log in through xdm, you will in all likelyhood get an empty
list.  But X has its own idea about keyboard mappings anyway.  It only
initializes its settings based on the wscons settings.  But it never
changes the wscons settings if you manipulate the X mappings.

Cheers,

Mark

Re: Fix kbd -l

[Chris Bennett]

Lucky day.
I brought the USB drive that I was running current amd64 on.

setxkbmap -layout "us,ru,latam"  -option caps:none -option grp:shifts_toggle 
-option grp_led:scroll

plus
.xinitrc   
/usr/X11R6/bin/xset b
/usr/X11R6/bin/xset fp+ /usr/local/share/fonts/terminus
/usr/X11R6/bin/xset fp+ /usr/local/share/fonts/ptsans/

Chris Bennett

Re: Fix kbd -l

[Theo Buehler]

On Thu, Sep 29, 2016 at 02:19:43PM +0200, Mark Kettenis wrote:
> This diff adds a WSKBDIO_GETENCODINGS ioctl and uses it to print a
> list of supported encodings like the old kvm groveling code did.  The
> ioctl will clamp the number of entries that are returns to the number
> that was passed in.  This means that if the number returned is the
> same as the number passed in, there might be more entries and you
> probably want to retry with a larger buffer.  The new implementation
> does this.
> 
> Waring; this did not go through a make release yet.  And that might be
> relevant, since this code gets included in the ramdisks.
> 
> Thoughts?
> 

amd64 release builds fine with this patch. I tested it on both a normal
system and with bsd.rd, works fine.  I like the fact that extended
keyboard encodings are now also listed in bsd.rd.

There is one change of behavior: previously, if a non-root user ran
kbd -l, it would exit with status 1 and print the error message:

kbd: kvm_openfiles: /dev/mem: permission denied

which gave at least some indication that it must be run as root.

Now it fails silently with exit code 0 without printing anything since
opening the /dev/wskbd?  devices fails, as they are owned by root:wheel
with permissions 600.

Perhaps it would be appropriate to error out before calling kbd_list()
if getuid() != 0.

> Index: sys/dev/wscons/wsconsio.h
> ===
> RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
> retrieving revision 1.75
> diff -u -p -r1.75 wsconsio.h
> --- sys/dev/wscons/wsconsio.h 14 Sep 2016 03:25:51 -  1.75
> +++ sys/dev/wscons/wsconsio.h 29 Sep 2016 12:10:57 -
> @@ -207,6 +207,12 @@ struct wskbd_backlight {
>  #define  WSKBD_TRANSLATED0
>  #define  WSKBD_RAW   1
>  
> +struct wskbd_encoding_data {
> + int nencodings;
> + kbd_t   *encodings;
> +};
> +#define WSKBDIO_GETENCODINGS _IOWR('W', 21, struct wskbd_encoding_data)
> +
>  /*
>   * Mouse ioctls (32 - 63)
>   */
> Index: sys/dev/wscons/wskbd.c
> ===
> RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 wskbd.c
> --- sys/dev/wscons/wskbd.c12 Dec 2015 12:30:18 -  1.83
> +++ sys/dev/wscons/wskbd.c29 Sep 2016 12:10:57 -
> @@ -1001,9 +1001,11 @@ wskbd_displayioctl(struct device *dev, u
>   struct wskbd_bell_data *ubdp, *kbdp;
>   struct wskbd_keyrepeat_data *ukdp, *kkdp;
>   struct wskbd_map_data *umdp;
> + struct wskbd_encoding_data *uedp;
>   kbd_t enc;
>   void *buf;
>   int len, error;
> + int count, i;
>  
>   switch (cmd) {
>   case WSKBDIO_BELL:
> @@ -1159,6 +1161,20 @@ getkeyrepeat:
>   wsmux_set_layout(sc->sc_base.me_parent, enc);
>  #endif
>   return (0);
> +
> + case WSKBDIO_GETENCODINGS:
> + uedp = (struct wskbd_encoding_data *)data;
> + for (count = 0; sc->id->t_keymap.keydesc[count].name; count++)
> + ;
> + if (uedp->nencodings > count)
> + uedp->nencodings = count;
> + for (i = 0; i < uedp->nencodings; i++) {
> + error = copyout(>id->t_keymap.keydesc[i].name,
> + >encodings[i], sizeof(kbd_t));
> + if (error)
> + return (error);
> + }
> + return(0);
>  
>   case WSKBDIO_GETBACKLIGHT:
>   if (wskbd_get_backlight != NULL)
> Index: sbin/kbd/kbd_wscons.c
> ===
> RCS file: /cvs/src/sbin/kbd/kbd_wscons.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 kbd_wscons.c
> --- sbin/kbd/kbd_wscons.c 27 Sep 2016 22:03:49 -  1.30
> +++ sbin/kbd/kbd_wscons.c 29 Sep 2016 12:10:57 -
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -82,31 +83,83 @@ struct nameint kbdvar_tab[] = {
>  
>  extern char *__progname;
>  
> -void kbd_show_enc(int idx);
> +void kbd_show_enc(struct wskbd_encoding_data *encs, int idx);
> +void kbd_get_encs(int fd, struct wskbd_encoding_data *encs);
>  void kbd_list(void);
>  void kbd_set(char *name, int verbose);
>  
>  void
> -kbd_show_enc(int idx)
> +kbd_show_enc(struct wskbd_encoding_data *encs, int idx)
>  {
> + int found;
> + kbd_t encoding, variant;
> + struct nameint *n;
>   int i;
>  
>   printf("tables available for %s keyboard:\nencoding\n\n",
>   kbtype_tab[idx]);
>  
> - for (i = 0; kbdenc_tab[i].value; i++)
> - printf("%s\n", kbdenc_tab[i].name);
> + for (i = 0; i < encs->nencodings; i++) {
> + n = _tab[0];
> + found = 0;
> + encoding = encs->encodings[i];
> + while (n->value) {
> + if (n->value == 

Re: Fix kbd -l

[Chris Bennett]

When I return to Mexico shortly, I found that during the steps I took to
setup a keyboard to be switchable between english-spanish-russian, there
was a problem with one program outputting 'en' but kbd -l giving 'us'.
Since I only have that set-up back there, I can't show you the file that
worked. pretty sure I used dual shift keys to switch keyboard encodings.

I'll look up the thread on misc@ that I asked about all this right now.
See if I can reproduce here.

Chris Bennett