diff mbox series

[07/26] qemu-keymap: properly check return from xkb_keymap_mod_get_index

Message ID 20230623122100.1640995-8-alex.bennee@linaro.org
State Superseded
Headers show
Series maintainer omnibus: testing, fuzz, plugins, documentation | expand

Commit Message

Alex Bennée June 23, 2023, 12:20 p.m. UTC 
We can return XKB_MOD_INVALID which rightly gets flagged by sanitisers
as an overly wide shift attempt.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 qemu-keymap.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Peter Maydell June 23, 2023, 12:25 p.m. UTC | #1 
On Fri, 23 Jun 2023 at 13:21, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> We can return XKB_MOD_INVALID which rightly gets flagged by sanitisers
> as an overly wide shift attempt.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Same comments as on the first version of this patch:
looks OK code-wise, but have you eyeballed the output?
Does the keyboard layout that triggers this have no
AltGr at all, or does it call it by a different name?

thanks
-- PMM
Alex Bennée June 26, 2023, 8:21 p.m. UTC | #2 
Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 23 Jun 2023 at 13:21, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> We can return XKB_MOD_INVALID which rightly gets flagged by sanitisers
>> as an overly wide shift attempt.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> Same comments as on the first version of this patch:
> looks OK code-wise, but have you eyeballed the output?

I've eyeballed it but practically it doesn't seem to make any difference
to the output:

  🕙21:20:36 alex@zen:qemu.git/builds/all  (399fc0c) (REBASING 2/22) [$!?] 
  ➜  diff -ub gb.before gb.after
  🕙21:20:43 alex@zen:qemu.git/builds/all  (399fc0c) (REBASING 2/22) [$!?] 
  ➜  diff -ub ara.before ara.after

> Does the keyboard layout that triggers this have no
> AltGr at all, or does it call it by a different name?

Certainly not ara or gb:

  9: Alt
23:#    11: LAlt
24:#    12: RAlt
29:#    17: AltGr
294:Alt_L 0x38
1711:Alt_R 0xb8
🕙21:22:14 alex@zen:qemu.git/builds/all  (399fc0c) (REBASING 2/22) [$!?] 
➜  ag "Alt" gb.after 
21:#     9: Alt
23:#    11: LAlt
24:#    12: RAlt
29:#    17: AltGr
338:Alt_L 0x38
1757:Alt_R 0xb8

>
> thanks
> -- PMM
diff mbox series

Patch

diff --git a/qemu-keymap.c b/qemu-keymap.c
index 229866e004..8c80f7a4ed 100644
--- a/qemu-keymap.c
+++ b/qemu-keymap.c
@@ -140,6 +140,18 @@  static void usage(FILE *out)
             names.options ?: "-");
 }
 
+static xkb_mod_mask_t get_mod(struct xkb_keymap *map, const char *name)
+{
+    xkb_mod_index_t mod;
+    xkb_mod_mask_t mask = 0;
+
+    mod = xkb_keymap_mod_get_index(map, name);
+    if (mod != XKB_MOD_INVALID) {
+        mask = (1 << mod);
+    }
+    return mask;
+}
+
 int main(int argc, char *argv[])
 {
     struct xkb_context *ctx;
@@ -215,14 +227,10 @@  int main(int argc, char *argv[])
                 mod, xkb_keymap_mod_get_name(map, mod));
     }
 
-    mod = xkb_keymap_mod_get_index(map, "Shift");
-    shift = (1 << mod);
-    mod = xkb_keymap_mod_get_index(map, "Control");
-    ctrl = (1 << mod);
-    mod = xkb_keymap_mod_get_index(map, "AltGr");
-    altgr = (1 << mod);
-    mod = xkb_keymap_mod_get_index(map, "NumLock");
-    numlock = (1 << mod);
+    shift = get_mod(map, "Shift");
+    ctrl = get_mod(map, "Control");
+    altgr = get_mod(map, "AltGr");
+    numlock = get_mod(map, "NumLock");
 
     state = xkb_state_new(map);
     xkb_keymap_key_for_each(map, walk_map, state);