[v1] maintainer omnibus: testing, fuzz, plugins, documentation

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

Alex Bennée posted 26 patches 2 years, 7 months ago

Diff against v2 v2
Download mbox

Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Bin Meng <bmeng.cn@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Riku Voipio <riku.voipio@iki.fi>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Juan Quintela <quintela@redhat.com>, Peter Xu <peterx@redhat.com>, Leonardo Bras <leobras@redhat.com>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Darren Kenny <darren.kenny@oracle.com>, Qiuhao Li <Qiuhao.Li@outlook.com>, Peter Maydell <peter.maydell@linaro.org>, Radoslaw Biernacki <rad@semihalf.com>, Leif Lindholm <quic_llindhol@quicinc.com>, Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>, Cleber Rosa <crosa@redhat.com>, Laurent Vivier <lvivier@redhat.com>

There is a newer version of this series

Expand all Fold all

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

Posted by Alex Bennée 2 years, 7 months ago

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(-)

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);
-- 
2.39.2

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

Posted by Peter Maydell 2 years, 7 months ago

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

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

Posted by Alex Bennée 2 years, 7 months ago

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


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro