[PULL,5/7] hw: convert the escc device to keycodemapdb

Commit Message

Gerd Hoffmann Jan. 29, 2018, 10:23 a.m. UTC

From: "Daniel P. Berrange" <[e-mail]>

Replace the qcode_to_keycode table with automatically
generated tables.

Missing entries in qcode_to_keycode now fixed:

 - Q_KEY_CODE_KP_COMMA -> 0x2d

Signed-off-by: Daniel P. Berrange <[e-mail]>
Message-id: [e-mail]
Signed-off-by: Gerd Hoffmann <[e-mail]>
---
 Makefile           |   1 +
 include/ui/input.h |   3 ++
 hw/char/escc.c     | 126 +++--------------------------------------------------
 ui/input-keymap.c  |   1 +
 4 files changed, 10 insertions(+), 121 deletions(-)

Comments

Peter Maydell March 31, 2022, 10:03 a.m. UTC | #1

On Mon, 29 Jan 2018 at 10:24, Gerd Hoffmann <[e-mail]> wrote:
>
> From: "Daniel P. Berrange" <[e-mail]>
>
> Replace the qcode_to_keycode table with automatically
> generated tables.
>
> Missing entries in qcode_to_keycode now fixed:
>
>  - Q_KEY_CODE_KP_COMMA -> 0x2d
>
> Signed-off-by: Daniel P. Berrange <[e-mail]>
> Message-id: [e-mail]
> Signed-off-by: Gerd Hoffmann <[e-mail]>

> @@ -879,7 +759,11 @@ static void sunkbd_handle_event(DeviceState *dev, QemuConsole *src,
>          }
>      }
>
> -    keycode = qcode_to_keycode[qcode];
> +    if (qcode > qemu_input_map_qcode_to_sun_len) {
> +        return;
> +    }
> +
> +    keycode = qemu_input_map_qcode_to_sun[qcode];
>      if (!key->down) {
>          keycode |= 0x80;
>      }

Hi; I was looking at this code because Coverity is now clever enough
to try to check whether the qemu_input_map_qcode_to_sun[] array
is being overrun (though alas not clever enough to spot that
qemu_input_map_qcode_to_sun_len is the length of that array,
so there are false positive complaints about all the uses of these
autogenerated arrays in all devices that use them).

In this specific case, though, it does look like there's a bug:
shouldn't the condition be "if (qcode >= qemu_input_map_qcode_to_sun_len)" ?

thanks
-- PMM

Daniel P. Berrangé March 31, 2022, 10:10 a.m. UTC | #2

On Thu, Mar 31, 2022 at 11:03:49AM +0100, Peter Maydell wrote:
> On Mon, 29 Jan 2018 at 10:24, Gerd Hoffmann <[e-mail]> wrote:
> >
> > From: "Daniel P. Berrange" <[e-mail]>
> >
> > Replace the qcode_to_keycode table with automatically
> > generated tables.
> >
> > Missing entries in qcode_to_keycode now fixed:
> >
> >  - Q_KEY_CODE_KP_COMMA -> 0x2d
> >
> > Signed-off-by: Daniel P. Berrange <[e-mail]>
> > Message-id: [e-mail]
> > Signed-off-by: Gerd Hoffmann <[e-mail]>
> 
> > @@ -879,7 +759,11 @@ static void sunkbd_handle_event(DeviceState *dev, QemuConsole *src,
> >          }
> >      }
> >
> > -    keycode = qcode_to_keycode[qcode];
> > +    if (qcode > qemu_input_map_qcode_to_sun_len) {
> > +        return;
> > +    }
> > +
> > +    keycode = qemu_input_map_qcode_to_sun[qcode];
> >      if (!key->down) {
> >          keycode |= 0x80;
> >      }
> 
> Hi; I was looking at this code because Coverity is now clever enough
> to try to check whether the qemu_input_map_qcode_to_sun[] array
> is being overrun (though alas not clever enough to spot that
> qemu_input_map_qcode_to_sun_len is the length of that array,
> so there are false positive complaints about all the uses of these
> autogenerated arrays in all devices that use them).
> 
> In this specific case, though, it does look like there's a bug:
> shouldn't the condition be "if (qcode >= qemu_input_map_qcode_to_sun_len)" ?

Yes, you are right.


With regards,
Daniel

Patch

diff --git a/Makefile b/Makefile
index cd65eb4a80..f9278f14c9 100644
--- a/Makefile
+++ b/Makefile
@@ -238,6 +238,7 @@  KEYCODEMAP_FILES = \
 		 ui/input-keymap-qcode-to-atset2.c \
 		 ui/input-keymap-qcode-to-atset3.c \
 		 ui/input-keymap-qcode-to-qnum.c \
+		 ui/input-keymap-qcode-to-sun.c \
 		 ui/input-keymap-qnum-to-qcode.c \
 		 ui/input-keymap-qcode-to-linux.c \
 		 ui/input-keymap-usb-to-qcode.c \
diff --git a/include/ui/input.h b/include/ui/input.h
index cffeb1c1a0..7d6c92224e 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -86,6 +86,9 @@  extern const guint16 qemu_input_map_qcode_to_atset3[];
 extern const guint qemu_input_map_qcode_to_qnum_len;
 extern const guint16 qemu_input_map_qcode_to_qnum[];
 
+extern const guint qemu_input_map_qcode_to_sun_len;
+extern const guint16 qemu_input_map_qcode_to_sun[];
+
 extern const guint qemu_input_map_qnum_to_qcode_len;
 extern const guint16 qemu_input_map_qnum_to_qcode[];
 
diff --git a/hw/char/escc.c b/hw/char/escc.c
index 3ab831a6a7..449bf2fc63 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -717,126 +717,6 @@  MemoryRegion *escc_init(hwaddr base, qemu_irq irqA, qemu_irq irqB,
     return &d->mmio;
 }
 
-static const uint8_t qcode_to_keycode[Q_KEY_CODE__MAX] = {
-    [Q_KEY_CODE_SHIFT]         = 99,
-    [Q_KEY_CODE_SHIFT_R]       = 110,
-    [Q_KEY_CODE_ALT]           = 19,
-    [Q_KEY_CODE_ALT_R]         = 13,
-    [Q_KEY_CODE_CTRL]          = 76,
-    [Q_KEY_CODE_CTRL_R]        = 76,
-    [Q_KEY_CODE_ESC]           = 29,
-    [Q_KEY_CODE_1]             = 30,
-    [Q_KEY_CODE_2]             = 31,
-    [Q_KEY_CODE_3]             = 32,
-    [Q_KEY_CODE_4]             = 33,
-    [Q_KEY_CODE_5]             = 34,
-    [Q_KEY_CODE_6]             = 35,
-    [Q_KEY_CODE_7]             = 36,
-    [Q_KEY_CODE_8]             = 37,
-    [Q_KEY_CODE_9]             = 38,
-    [Q_KEY_CODE_0]             = 39,
-    [Q_KEY_CODE_MINUS]         = 40,
-    [Q_KEY_CODE_EQUAL]         = 41,
-    [Q_KEY_CODE_BACKSPACE]     = 43,
-    [Q_KEY_CODE_TAB]           = 53,
-    [Q_KEY_CODE_Q]             = 54,
-    [Q_KEY_CODE_W]             = 55,
-    [Q_KEY_CODE_E]             = 56,
-    [Q_KEY_CODE_R]             = 57,
-    [Q_KEY_CODE_T]             = 58,
-    [Q_KEY_CODE_Y]             = 59,
-    [Q_KEY_CODE_U]             = 60,
-    [Q_KEY_CODE_I]             = 61,
-    [Q_KEY_CODE_O]             = 62,
-    [Q_KEY_CODE_P]             = 63,
-    [Q_KEY_CODE_BRACKET_LEFT]  = 64,
-    [Q_KEY_CODE_BRACKET_RIGHT] = 65,
-    [Q_KEY_CODE_RET]           = 89,
-    [Q_KEY_CODE_A]             = 77,
-    [Q_KEY_CODE_S]             = 78,
-    [Q_KEY_CODE_D]             = 79,
-    [Q_KEY_CODE_F]             = 80,
-    [Q_KEY_CODE_G]             = 81,
-    [Q_KEY_CODE_H]             = 82,
-    [Q_KEY_CODE_J]             = 83,
-    [Q_KEY_CODE_K]             = 84,
-    [Q_KEY_CODE_L]             = 85,
-    [Q_KEY_CODE_SEMICOLON]     = 86,
-    [Q_KEY_CODE_APOSTROPHE]    = 87,
-    [Q_KEY_CODE_GRAVE_ACCENT]  = 42,
-    [Q_KEY_CODE_BACKSLASH]     = 88,
-    [Q_KEY_CODE_Z]             = 100,
-    [Q_KEY_CODE_X]             = 101,
-    [Q_KEY_CODE_C]             = 102,
-    [Q_KEY_CODE_V]             = 103,
-    [Q_KEY_CODE_B]             = 104,
-    [Q_KEY_CODE_N]             = 105,
-    [Q_KEY_CODE_M]             = 106,
-    [Q_KEY_CODE_COMMA]         = 107,
-    [Q_KEY_CODE_DOT]           = 108,
-    [Q_KEY_CODE_SLASH]         = 109,
-    [Q_KEY_CODE_ASTERISK]      = 47,
-    [Q_KEY_CODE_SPC]           = 121,
-    [Q_KEY_CODE_CAPS_LOCK]     = 119,
-    [Q_KEY_CODE_F1]            = 5,
-    [Q_KEY_CODE_F2]            = 6,
-    [Q_KEY_CODE_F3]            = 8,
-    [Q_KEY_CODE_F4]            = 10,
-    [Q_KEY_CODE_F5]            = 12,
-    [Q_KEY_CODE_F6]            = 14,
-    [Q_KEY_CODE_F7]            = 16,
-    [Q_KEY_CODE_F8]            = 17,
-    [Q_KEY_CODE_F9]            = 18,
-    [Q_KEY_CODE_F10]           = 7,
-    [Q_KEY_CODE_NUM_LOCK]      = 98,
-    [Q_KEY_CODE_SCROLL_LOCK]   = 23,
-    [Q_KEY_CODE_KP_DIVIDE]     = 46,
-    [Q_KEY_CODE_KP_MULTIPLY]   = 47,
-    [Q_KEY_CODE_KP_SUBTRACT]   = 71,
-    [Q_KEY_CODE_KP_ADD]        = 125,
-    [Q_KEY_CODE_KP_ENTER]      = 90,
-    [Q_KEY_CODE_KP_DECIMAL]    = 50,
-    [Q_KEY_CODE_KP_0]          = 94,
-    [Q_KEY_CODE_KP_1]          = 112,
-    [Q_KEY_CODE_KP_2]          = 113,
-    [Q_KEY_CODE_KP_3]          = 114,
-    [Q_KEY_CODE_KP_4]          = 91,
-    [Q_KEY_CODE_KP_5]          = 92,
-    [Q_KEY_CODE_KP_6]          = 93,
-    [Q_KEY_CODE_KP_7]          = 68,
-    [Q_KEY_CODE_KP_8]          = 69,
-    [Q_KEY_CODE_KP_9]          = 70,
-    [Q_KEY_CODE_LESS]          = 124,
-    [Q_KEY_CODE_F11]           = 9,
-    [Q_KEY_CODE_F12]           = 11,
-    [Q_KEY_CODE_HOME]          = 52,
-    [Q_KEY_CODE_PGUP]          = 96,
-    [Q_KEY_CODE_PGDN]          = 123,
-    [Q_KEY_CODE_END]           = 74,
-    [Q_KEY_CODE_LEFT]          = 24,
-    [Q_KEY_CODE_UP]            = 20,
-    [Q_KEY_CODE_DOWN]          = 27,
-    [Q_KEY_CODE_RIGHT]         = 28,
-    [Q_KEY_CODE_INSERT]        = 44,
-    [Q_KEY_CODE_DELETE]        = 66,
-    [Q_KEY_CODE_STOP]          = 1,
-    [Q_KEY_CODE_AGAIN]         = 3,
-    [Q_KEY_CODE_PROPS]         = 25,
-    [Q_KEY_CODE_UNDO]          = 26,
-    [Q_KEY_CODE_FRONT]         = 49,
-    [Q_KEY_CODE_COPY]          = 51,
-    [Q_KEY_CODE_OPEN]          = 72,
-    [Q_KEY_CODE_PASTE]         = 73,
-    [Q_KEY_CODE_FIND]          = 95,
-    [Q_KEY_CODE_CUT]           = 97,
-    [Q_KEY_CODE_LF]            = 111,
-    [Q_KEY_CODE_HELP]          = 118,
-    [Q_KEY_CODE_META_L]        = 120,
-    [Q_KEY_CODE_META_R]        = 122,
-    [Q_KEY_CODE_COMPOSE]       = 67,
-    [Q_KEY_CODE_PRINT]         = 22,
-    [Q_KEY_CODE_SYSRQ]         = 21,
-};
 
 static void sunkbd_handle_event(DeviceState *dev, QemuConsole *src,
                                 InputEvent *evt)
@@ -879,7 +759,11 @@  static void sunkbd_handle_event(DeviceState *dev, QemuConsole *src,
         }
     }
 
-    keycode = qcode_to_keycode[qcode];
+    if (qcode > qemu_input_map_qcode_to_sun_len) {
+        return;
+    }
+
+    keycode = qemu_input_map_qcode_to_sun[qcode];
     if (!key->down) {
         keycode |= 0x80;
     }
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index 07be8f4415..7a9475abd9 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -11,6 +11,7 @@ 
 #include "ui/input-keymap-qcode-to-atset2.c"
 #include "ui/input-keymap-qcode-to-atset3.c"
 #include "ui/input-keymap-qcode-to-qnum.c"
+#include "ui/input-keymap-qcode-to-sun.c"
 #include "ui/input-keymap-qnum-to-qcode.c"
 #include "ui/input-keymap-qcode-to-linux.c"
 #include "ui/input-keymap-usb-to-qcode.c"