[Qemu-devel] [PATCH v2 0/5] keymap: support kbd layouts with multiple mappings for the same key.

[Qemu-devel] [PATCH v2 0/5] keymap: support kbd layouts with multiple mappings for the same key.

[Gerd Hoffmann]

The reverse keymap (-k $map) code has problems dealing with keymaps
where multiple key combinations can create the same keysym, because
it can store a single keycode per keysym only.  This series fixes it
and does some cleanups along the way.

v2: rebase, codestyle fixes.

Gerd Hoffmann (5):
  keymap: make struct kbd_layout_t private to ui/keymaps.c
  keymap: use glib hash for kbd_layout_t
  keymap: numpad keysyms and keycodes are fixed
  keymap: record multiple keysym -> keycode mappings
  keymap: consider modifier state when picking a mapping

 ui/keymaps.h    |  30 +++-------
 ui/curses.c     |   3 +-
 ui/keymaps.c    | 169 +++++++++++++++++++++++++++++---------------------------
 ui/sdl.c        |   6 +-
 ui/vnc.c        |   9 ++-
 ui/trace-events |   2 +-
 6 files changed, 111 insertions(+), 108 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/5] keymap: make struct kbd_layout_t private to ui/keymaps.c

[Gerd Hoffmann]

Also use kbd_layout_t pointers instead of void pointers.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/keymaps.h | 29 ++++++-----------------------
 ui/keymaps.c | 32 +++++++++++++++++++++++++-------
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/ui/keymaps.h b/ui/keymaps.h
index 8757465529..17ec03387a 100644
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -32,25 +32,6 @@ typedef struct {
 	int keysym;
 } name2keysym_t;
 
-struct key_range {
-    int start;
-    int end;
-    struct key_range *next;
-};
-
-#define MAX_NORMAL_KEYCODE 512
-#define MAX_EXTRA_COUNT 256
-typedef struct {
-    uint16_t keysym2keycode[MAX_NORMAL_KEYCODE];
-    struct {
-	int keysym;
-	uint16_t keycode;
-    } keysym2keycode_extra[MAX_EXTRA_COUNT];
-    int extra_count;
-    struct key_range *keypad_range;
-    struct key_range *numlock_range;
-} kbd_layout_t;
-
 /* scancode without modifiers */
 #define SCANCODE_KEYMASK 0xff
 /* scancode without grey or up bit */
@@ -69,10 +50,12 @@ typedef struct {
 #define SCANCODE_ALT    0x400
 #define SCANCODE_ALTGR  0x800
 
+typedef struct kbd_layout_t kbd_layout_t;
 
-void *init_keyboard_layout(const name2keysym_t *table, const char *language);
-int keysym2scancode(void *kbd_layout, int keysym);
-int keycode_is_keypad(void *kbd_layout, int keycode);
-int keysym_is_numlock(void *kbd_layout, int keysym);
+kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
+                                   const char *language);
+int keysym2scancode(kbd_layout_t *k, int keysym);
+int keycode_is_keypad(kbd_layout_t *k, int keycode);
+int keysym_is_numlock(kbd_layout_t *k, int keysym);
 
 #endif /* QEMU_KEYMAPS_H */
diff --git a/ui/keymaps.c b/ui/keymaps.c
index f9762d1497..134958a197 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,6 +28,26 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 
+#define MAX_NORMAL_KEYCODE 512
+#define MAX_EXTRA_COUNT 256
+
+struct key_range {
+    int start;
+    int end;
+    struct key_range *next;
+};
+
+struct kbd_layout_t {
+    uint16_t keysym2keycode[MAX_NORMAL_KEYCODE];
+    struct {
+        int keysym;
+        uint16_t keycode;
+    } keysym2keycode_extra[MAX_EXTRA_COUNT];
+    int extra_count;
+    struct key_range *keypad_range;
+    struct key_range *numlock_range;
+};
+
 static int get_keysym(const name2keysym_t *table,
                       const char *name)
 {
@@ -186,15 +206,15 @@ static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
 }
 
 
-void *init_keyboard_layout(const name2keysym_t *table, const char *language)
+kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
+                                   const char *language)
 {
     return parse_keyboard_layout(table, language, NULL);
 }
 
 
-int keysym2scancode(void *kbd_layout, int keysym)
+int keysym2scancode(kbd_layout_t *k, int keysym)
 {
-    kbd_layout_t *k = kbd_layout;
     if (keysym < MAX_NORMAL_KEYCODE) {
         if (k->keysym2keycode[keysym] == 0) {
             trace_keymap_unmapped(keysym);
@@ -217,9 +237,8 @@ int keysym2scancode(void *kbd_layout, int keysym)
     return 0;
 }
 
-int keycode_is_keypad(void *kbd_layout, int keycode)
+int keycode_is_keypad(kbd_layout_t *k, int keycode)
 {
-    kbd_layout_t *k = kbd_layout;
     struct key_range *kr;
 
     for (kr = k->keypad_range; kr; kr = kr->next) {
@@ -230,9 +249,8 @@ int keycode_is_keypad(void *kbd_layout, int keycode)
     return 0;
 }
 
-int keysym_is_numlock(void *kbd_layout, int keysym)
+int keysym_is_numlock(kbd_layout_t *k, int keysym)
 {
-    kbd_layout_t *k = kbd_layout;
     struct key_range *kr;
 
     for (kr = k->numlock_range; kr; kr = kr->next) {
-- 
2.9.3

[Qemu-devel] [PATCH v2 2/5] keymap: use glib hash for kbd_layout_t

[Gerd Hoffmann]

Drop home-grown lookup code, which is a strange mix of a lookup table
and a list.  Use standard glib hash instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/keymaps.c    | 79 +++++++++++++++++++++++++++------------------------------
 ui/trace-events |  2 +-
 2 files changed, 38 insertions(+), 43 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index 134958a197..bef1405576 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,26 +28,28 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 
-#define MAX_NORMAL_KEYCODE 512
-#define MAX_EXTRA_COUNT 256
-
 struct key_range {
     int start;
     int end;
     struct key_range *next;
 };
 
+struct keysym2code {
+    uint16_t keycode;
+};
+
 struct kbd_layout_t {
-    uint16_t keysym2keycode[MAX_NORMAL_KEYCODE];
-    struct {
-        int keysym;
-        uint16_t keycode;
-    } keysym2keycode_extra[MAX_EXTRA_COUNT];
-    int extra_count;
+    GHashTable *hash;
     struct key_range *keypad_range;
     struct key_range *numlock_range;
 };
 
+static inline gpointer hashkey(int keysym)
+{
+    intptr_t ptr = keysym;
+    return (gpointer)(ptr);
+}
+
 static int get_keysym(const name2keysym_t *table,
                       const char *name)
 {
@@ -91,23 +93,19 @@ static void add_to_key_range(struct key_range **krp, int code) {
     }
 }
 
-static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k) {
-    if (keysym < MAX_NORMAL_KEYCODE) {
-        trace_keymap_add("normal", keysym, keycode, line);
-        k->keysym2keycode[keysym] = keycode;
-    } else {
-        if (k->extra_count >= MAX_EXTRA_COUNT) {
-            warn_report("Could not assign keysym %s (0x%x)"
-                        " because of memory constraints.", line, keysym);
-        } else {
-            trace_keymap_add("extra", keysym, keycode, line);
-            k->keysym2keycode_extra[k->extra_count].
-            keysym = keysym;
-            k->keysym2keycode_extra[k->extra_count].
-            keycode = keycode;
-            k->extra_count++;
-        }
+static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k)
+{
+    struct keysym2code *keysym2code;
+
+    keysym2code = g_hash_table_lookup(k->hash, hashkey(keysym));
+    if (keysym2code) {
+        return;
     }
+
+    keysym2code = g_new0(struct keysym2code, 1);
+    keysym2code->keycode = keycode;
+    g_hash_table_replace(k->hash, hashkey(keysym), keysym2code);
+    trace_keymap_add(keysym, keycode, line);
 }
 
 static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
@@ -131,6 +129,7 @@ static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
 
     if (!k) {
         k = g_new0(kbd_layout_t, 1);
+        k->hash = g_hash_table_new(NULL, NULL);
     }
 
     for(;;) {
@@ -215,26 +214,22 @@ kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
 
 int keysym2scancode(kbd_layout_t *k, int keysym)
 {
-    if (keysym < MAX_NORMAL_KEYCODE) {
-        if (k->keysym2keycode[keysym] == 0) {
-            trace_keymap_unmapped(keysym);
-            warn_report("no scancode found for keysym %d", keysym);
-        }
-        return k->keysym2keycode[keysym];
-    } else {
-        int i;
+    struct keysym2code *keysym2code;
+
 #ifdef XK_ISO_Left_Tab
-        if (keysym == XK_ISO_Left_Tab) {
-            keysym = XK_Tab;
-        }
+    if (keysym == XK_ISO_Left_Tab) {
+        keysym = XK_Tab;
+    }
 #endif
-        for (i = 0; i < k->extra_count; i++) {
-            if (k->keysym2keycode_extra[i].keysym == keysym) {
-                return k->keysym2keycode_extra[i].keycode;
-            }
-        }
+
+    keysym2code = g_hash_table_lookup(k->hash, hashkey(keysym));
+    if (!keysym2code) {
+        trace_keymap_unmapped(keysym);
+        warn_report("no scancode found for keysym %d", keysym);
+        return 0;
     }
-    return 0;
+
+    return keysym2code->keycode;
 }
 
 int keycode_is_keypad(kbd_layout_t *k, int keycode)
diff --git a/ui/trace-events b/ui/trace-events
index 34229e6747..861b68a305 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -78,7 +78,7 @@ qemu_spice_create_update(uint32_t left, uint32_t right, uint32_t top, uint32_t b
 
 # ui/keymaps.c
 keymap_parse(const char *file) "file %s"
-keymap_add(const char *type, int sym, int code, const char *line) "%-6s sym=0x%04x code=0x%04x (line: %s)"
+keymap_add(int sym, int code, const char *line) "sym=0x%04x code=0x%04x (line: %s)"
 keymap_unmapped(int sym) "sym=0x%04x"
 
 # ui/x_keymap.c
-- 
2.9.3

[Qemu-devel] [PATCH v2 3/5] keymap: numpad keysyms and keycodes are fixed

[Gerd Hoffmann]

No need to figure them at runtime from the keymap.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/keymaps.c | 61 +++++++++---------------------------------------------------
 1 file changed, 9 insertions(+), 52 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index bef1405576..d71ae389e0 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,20 +28,12 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 
-struct key_range {
-    int start;
-    int end;
-    struct key_range *next;
-};
-
 struct keysym2code {
     uint16_t keycode;
 };
 
 struct kbd_layout_t {
     GHashTable *hash;
-    struct key_range *keypad_range;
-    struct key_range *numlock_range;
 };
 
 static inline gpointer hashkey(int keysym)
@@ -70,29 +62,6 @@ static int get_keysym(const name2keysym_t *table,
 }
 
 
-static void add_to_key_range(struct key_range **krp, int code) {
-    struct key_range *kr;
-    for (kr = *krp; kr; kr = kr->next) {
-        if (code >= kr->start && code <= kr->end) {
-            break;
-        }
-        if (code == kr->start - 1) {
-            kr->start--;
-            break;
-        }
-        if (code == kr->end + 1) {
-            kr->end++;
-            break;
-        }
-    }
-    if (kr == NULL) {
-        kr = g_malloc0(sizeof(*kr));
-        kr->start = kr->end = code;
-        kr->next = *krp;
-        *krp = kr;
-    }
-}
-
 static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k)
 {
     struct keysym2code *keysym2code;
@@ -166,13 +135,6 @@ static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
                     const char *rest = line + offset + 1;
                     int keycode = strtol(rest, NULL, 0);
 
-                    if (strstr(rest, "numlock")) {
-                        add_to_key_range(&k->keypad_range, keycode);
-                        add_to_key_range(&k->numlock_range, keysym);
-                        /* fprintf(stderr, "keypad keysym %04x keycode %d\n",
-                                   keysym, keycode); */
-                    }
-
                     if (strstr(rest, "shift")) {
                         keycode |= SCANCODE_SHIFT;
                     }
@@ -234,24 +196,19 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
 
 int keycode_is_keypad(kbd_layout_t *k, int keycode)
 {
-    struct key_range *kr;
-
-    for (kr = k->keypad_range; kr; kr = kr->next) {
-        if (keycode >= kr->start && keycode <= kr->end) {
-            return 1;
-        }
+    if (keycode >= 0x47 && keycode <= 0x53) {
+        return true;
     }
-    return 0;
+    return false;
 }
 
 int keysym_is_numlock(kbd_layout_t *k, int keysym)
 {
-    struct key_range *kr;
-
-    for (kr = k->numlock_range; kr; kr = kr->next) {
-        if (keysym >= kr->start && keysym <= kr->end) {
-            return 1;
-        }
+    switch (keysym) {
+    case 0xffb0 ... 0xffb9:  /* KP_0 .. KP_9 */
+    case 0xffac:             /* KP_Separator */
+    case 0xffae:             /* KP_Decimal   */
+        return true;
     }
-    return 0;
+    return false;
 }
-- 
2.9.3

[Qemu-devel] [PATCH v2 4/5] keymap: record multiple keysym -> keycode mappings

[Gerd Hoffmann]

Sometimes the same keysym can be created using different key
combinations.  Record them all in the reverse keymap, not only
the first one.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/keymaps.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index d71ae389e0..26a85ff207 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -29,7 +29,8 @@
 #include "qemu/error-report.h"
 
 struct keysym2code {
-    uint16_t keycode;
+    uint32_t count;
+    uint16_t keycodes[4];
 };
 
 struct kbd_layout_t {
@@ -68,11 +69,18 @@ static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k)
 
     keysym2code = g_hash_table_lookup(k->hash, hashkey(keysym));
     if (keysym2code) {
+        if (keysym2code->count < ARRAY_SIZE(keysym2code->keycodes)) {
+            keysym2code->keycodes[keysym2code->count++] = keycode;
+        } else {
+            warn_report("more than %zd keycodes for keysym %d",
+                        ARRAY_SIZE(keysym2code->keycodes), keysym);
+        }
         return;
     }
 
     keysym2code = g_new0(struct keysym2code, 1);
-    keysym2code->keycode = keycode;
+    keysym2code->keycodes[0] = keycode;
+    keysym2code->count = 1;
     g_hash_table_replace(k->hash, hashkey(keysym), keysym2code);
     trace_keymap_add(keysym, keycode, line);
 }
@@ -191,7 +199,7 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
         return 0;
     }
 
-    return keysym2code->keycode;
+    return keysym2code->keycodes[0];
 }
 
 int keycode_is_keypad(kbd_layout_t *k, int keycode)
-- 
2.9.3

[Qemu-devel] [PATCH v2 5/5] keymap: consider modifier state when picking a mapping

[Gerd Hoffmann]

Pass the modifier state to the keymap lookup function.  In case multiple
keysym -> keycode mappings exist look at the modifier state and prefer
the mapping where the modifier state matches.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/keymaps.h |  3 ++-
 ui/curses.c  |  3 ++-
 ui/keymaps.c | 33 ++++++++++++++++++++++++++++++++-
 ui/sdl.c     |  6 +++++-
 ui/vnc.c     |  9 +++++++--
 5 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/ui/keymaps.h b/ui/keymaps.h
index 17ec03387a..0693588225 100644
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -54,7 +54,8 @@ typedef struct kbd_layout_t kbd_layout_t;
 
 kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
                                    const char *language);
-int keysym2scancode(kbd_layout_t *k, int keysym);
+int keysym2scancode(kbd_layout_t *k, int keysym,
+                    bool shift, bool altgr, bool ctrl);
 int keycode_is_keypad(kbd_layout_t *k, int keycode);
 int keysym_is_numlock(kbd_layout_t *k, int keysym);
 
diff --git a/ui/curses.c b/ui/curses.c
index 85503876c0..31cd43e73c 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -271,7 +271,8 @@ static void curses_refresh(DisplayChangeListener *dcl)
                     keysym = chr;
             }
 
-            keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK);
+            keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK,
+                                      false, false, false);
             if (keycode == 0)
                 continue;
 
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 26a85ff207..a10f4b7a67 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -182,8 +182,12 @@ kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
 }
 
 
-int keysym2scancode(kbd_layout_t *k, int keysym)
+int keysym2scancode(kbd_layout_t *k, int keysym,
+                    bool shift, bool altgr, bool ctrl)
 {
+    static const uint32_t mask =
+        SCANCODE_SHIFT | SCANCODE_ALTGR | SCANCODE_CTRL;
+    uint32_t mods, i;
     struct keysym2code *keysym2code;
 
 #ifdef XK_ISO_Left_Tab
@@ -199,6 +203,33 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
         return 0;
     }
 
+    if (keysym2code->count == 1) {
+        return keysym2code->keycodes[0];
+    }
+
+    /*
+     * We have multiple keysym -> keycode mappings.
+     *
+     * Check whenever we find one mapping where the modifier state of
+     * the mapping matches the current user interface modifier state.
+     * If so, prefer that one.
+     */
+    mods = 0;
+    if (shift) {
+        mods |= SCANCODE_SHIFT;
+    }
+    if (altgr) {
+        mods |= SCANCODE_ALTGR;
+    }
+    if (ctrl) {
+        mods |= SCANCODE_CTRL;
+    }
+
+    for (i = 0; i < keysym2code->count; i++) {
+        if ((keysym2code->keycodes[i] & mask) == mods) {
+            return keysym2code->keycodes[i];
+        }
+    }
     return keysym2code->keycodes[0];
 }
 
diff --git a/ui/sdl.c b/ui/sdl.c
index a6bff301eb..4c46c40a7f 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -200,6 +200,9 @@ static kbd_layout_t *kbd_layout = NULL;
 
 static uint8_t sdl_keyevent_to_keycode_generic(const SDL_KeyboardEvent *ev)
 {
+    bool shift = modifiers_state[0x2a] || modifiers_state[0x36];
+    bool altgr = modifiers_state[0xb8];
+    bool ctrl  = modifiers_state[0x1d] || modifiers_state[0x9d];
     int keysym;
     /* workaround for X11+SDL bug with AltGR */
     keysym = ev->keysym.sym;
@@ -209,7 +212,8 @@ static uint8_t sdl_keyevent_to_keycode_generic(const SDL_KeyboardEvent *ev)
     if (keysym == 92 && ev->keysym.scancode == 133) {
         keysym = 0xa5;
     }
-    return keysym2scancode(kbd_layout, keysym) & SCANCODE_KEYMASK;
+    return keysym2scancode(kbd_layout, keysym,
+                           shift, altgr, ctrl) & SCANCODE_KEYMASK;
 }
 
 
diff --git a/ui/vnc.c b/ui/vnc.c
index a77b568b57..d19f86c7f4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1734,7 +1734,8 @@ static void reset_keys(VncState *vs)
 
 static void press_key(VncState *vs, int keysym)
 {
-    int keycode = keysym2scancode(vs->vd->kbd_layout, keysym) & SCANCODE_KEYMASK;
+    int keycode = keysym2scancode(vs->vd->kbd_layout, keysym,
+                                  false, false, false) & SCANCODE_KEYMASK;
     qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, true);
     qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
     qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
@@ -1993,6 +1994,9 @@ static const char *code2name(int keycode)
 
 static void key_event(VncState *vs, int down, uint32_t sym)
 {
+    bool shift = vs->modifiers_state[0x2a] || vs->modifiers_state[0x36];
+    bool altgr = vs->modifiers_state[0xb8];
+    bool ctrl  = vs->modifiers_state[0x1d] || vs->modifiers_state[0x9d];
     int keycode;
     int lsym = sym;
 
@@ -2000,7 +2004,8 @@ static void key_event(VncState *vs, int down, uint32_t sym)
         lsym = lsym - 'A' + 'a';
     }
 
-    keycode = keysym2scancode(vs->vd->kbd_layout, lsym & 0xFFFF) & SCANCODE_KEYMASK;
+    keycode = keysym2scancode(vs->vd->kbd_layout, lsym & 0xFFFF,
+                              shift, altgr, ctrl) & SCANCODE_KEYMASK;
     trace_vnc_key_event_map(down, sym, keycode, code2name(keycode));
     do_key_event(vs, down, keycode, sym);
 }
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 1/5] keymap: make struct kbd_layout_t private to ui/keymaps.c

[Daniel P. Berrangé]

On Tue, Feb 20, 2018 at 04:03:53PM +0100, Gerd Hoffmann wrote:
> Also use kbd_layout_t pointers instead of void pointers.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/keymaps.h | 29 ++++++-----------------------
>  ui/keymaps.c | 32 +++++++++++++++++++++++++-------
>  2 files changed, 31 insertions(+), 30 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/5] keymap: use glib hash for kbd_layout_t

[Daniel P. Berrangé]

On Tue, Feb 20, 2018 at 04:03:54PM +0100, Gerd Hoffmann wrote:
> Drop home-grown lookup code, which is a strange mix of a lookup table
> and a list.  Use standard glib hash instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/keymaps.c    | 79 +++++++++++++++++++++++++++------------------------------
>  ui/trace-events |  2 +-
>  2 files changed, 38 insertions(+), 43 deletions(-)


> 
> diff --git a/ui/keymaps.c b/ui/keymaps.c
> index 134958a197..bef1405576 100644
> --- a/ui/keymaps.c
> +++ b/ui/keymaps.c
> @@ -28,26 +28,28 @@
>  #include "trace.h"
>  #include "qemu/error-report.h"
>  
> -#define MAX_NORMAL_KEYCODE 512
> -#define MAX_EXTRA_COUNT 256
> -
>  struct key_range {
>      int start;
>      int end;
>      struct key_range *next;
>  };
>  
> +struct keysym2code {
> +    uint16_t keycode;
> +};
> +
>  struct kbd_layout_t {
> -    uint16_t keysym2keycode[MAX_NORMAL_KEYCODE];
> -    struct {
> -        int keysym;
> -        uint16_t keycode;
> -    } keysym2keycode_extra[MAX_EXTRA_COUNT];
> -    int extra_count;
> +    GHashTable *hash;
>      struct key_range *keypad_range;
>      struct key_range *numlock_range;
>  };
>  
> +static inline gpointer hashkey(int keysym)
> +{
> +    intptr_t ptr = keysym;
> +    return (gpointer)(ptr);
> +}
> +
>  static int get_keysym(const name2keysym_t *table,
>                        const char *name)
>  {
> @@ -91,23 +93,19 @@ static void add_to_key_range(struct key_range **krp, int code) {
>      }
>  }
>  
> -static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k) {
> -    if (keysym < MAX_NORMAL_KEYCODE) {
> -        trace_keymap_add("normal", keysym, keycode, line);
> -        k->keysym2keycode[keysym] = keycode;
> -    } else {
> -        if (k->extra_count >= MAX_EXTRA_COUNT) {
> -            warn_report("Could not assign keysym %s (0x%x)"
> -                        " because of memory constraints.", line, keysym);
> -        } else {
> -            trace_keymap_add("extra", keysym, keycode, line);
> -            k->keysym2keycode_extra[k->extra_count].
> -            keysym = keysym;
> -            k->keysym2keycode_extra[k->extra_count].
> -            keycode = keycode;
> -            k->extra_count++;
> -        }
> +static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k)
> +{
> +    struct keysym2code *keysym2code;
> +
> +    keysym2code = g_hash_table_lookup(k->hash, hashkey(keysym));

FYI, the glib2 GINT_TO_POINTER() macro is intended to cover this
usecase of storing integers as hash keys. 


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 3/5] keymap: numpad keysyms and keycodes are fixed

[Daniel P. Berrangé]

On Tue, Feb 20, 2018 at 04:03:55PM +0100, Gerd Hoffmann wrote:
> No need to figure them at runtime from the keymap.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/keymaps.c | 61 +++++++++---------------------------------------------------
>  1 file changed, 9 insertions(+), 52 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 4/5] keymap: record multiple keysym -> keycode mappings

[Daniel P. Berrangé]

On Tue, Feb 20, 2018 at 04:03:56PM +0100, Gerd Hoffmann wrote:
> Sometimes the same keysym can be created using different key
> combinations.  Record them all in the reverse keymap, not only
> the first one.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/keymaps.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 5/5] keymap: consider modifier state when picking a mapping

[Daniel P. Berrangé]

On Tue, Feb 20, 2018 at 04:03:57PM +0100, Gerd Hoffmann wrote:
> Pass the modifier state to the keymap lookup function.  In case multiple
> keysym -> keycode mappings exist look at the modifier state and prefer
> the mapping where the modifier state matches.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/keymaps.h |  3 ++-
>  ui/curses.c  |  3 ++-
>  ui/keymaps.c | 33 ++++++++++++++++++++++++++++++++-
>  ui/sdl.c     |  6 +++++-
>  ui/vnc.c     |  9 +++++++--
>  5 files changed, 48 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|