Wskbd constness (Was: Patching wscons_keydesc at runtime)

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

On Sat, Jun 10, 2017 at 05:18:16 +0200, Emmanuel Dreyfus wrote:

> I just upgraded an HP Jornada 720 from NetBSD 2.0 to NetBSD 7.1, and
> discovered the wscons keymaps were broken in the meantime: it is impossible to
> change the keymap using wsconsctl encoding or wsconsctl map. Both commands
> succeed but have no effect.
> 
> After poking a few printf in the kernel, I found this in
> src/sys/dev/hpc/hpckbd.c:
> 
>         /* fix keydesc table */
>         /* 
>          * XXX The way this is done is really wrong.  The __UNCONST()
>          * is a hint as to what is wrong.  This actually ends up modifying
>          * initialized data which is marked "const".
>          * The reason we get away with it here is apparently that text
>          * and read-only data gets mapped read/write on the platforms
>          * using this code.
>          */
>         desc = (struct wscons_keydesc *)__UNCONST(hpckbd_keymapdata.keydesc);
>         for (i = 0; desc[i].name != 0; i++) {
>                 if ((desc[i].name & KB_MACHDEP) && desc[i].map == NULL) {
>                         desc[i].map = map;
>                         desc[i].map_size = mapsize;
>                 }
>         }
> 
> I managed to restore wscons keymaps by copying hpckbd_keymapdata.keydesc into
> a malloc() buffer and changing the hpckbd_keymapdata.keydesc to the new
> location, which is mapped read/write. 
> 
> The offending code did not change since NetBSD 2.0, except the XXX comment
> added in 2015. That suggests the compiler behavior changed about initalized
> const data, which was still mapped R/W in the ancient time and is now really
> read-only, altough it accepts nilpotent writes without raising an exception.

The constness in the MI wskbd code looks wrong:

    /* KBD_NULLMAP generates a entry for machine native variant.
       the entry will be modified by machine dependent keyboard driver. */
    #define KBD_NULLMAP() ...

    const struct wscons_keydesc pckbd_keydesctab[] = {
    ...
            /* placeholders */
            KBD_NULLMAP(KB_US | KB_MACHDEP, KB_US),
    ...
    };

Which is obviously self-contradictory.


This is probably b/c induced by const in:

    struct wskbd_mapdata {
            const struct wscons_keydesc *keydesc;
            kbd_t layout;
    };



> +               for (sz = 0; hpckbd_keymapdata.keydesc[sz].name != 0; sz++);

/usr/share/misc/style requires explicit no-op "continue" here.


-uwe