| Message ID | 20230626215926.2522656-7-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | maintainer omnibus: testing, fuzz, plugins, documentation | expand |
On 26/06/2023 23.59, Alex Bennée 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> > --- > qemu-keymap.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) Reviewed-by: Thomas Huth <thuth@redhat.com>
On 6/26/23 23:59, Alex Bennée 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> > --- > 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; > +} You have yet to answer Peter's question -- asked twice -- about what changes in the keymaps with this. If nothing, should it in fact be an assert instead? r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 6/26/23 23:59, Alex Bennée 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> >> --- >> 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; >> +} > > You have yet to answer Peter's question -- asked twice -- about what > changes in the keymaps with this. If nothing, should it in fact be an > assert instead? Ahh in the other thread. No change, it looks like AltGr just doesn't exist for some keymaps: 🕙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 🕙21:20:50 alex@zen:qemu.git/builds/all (399fc0c) (REBASING 2/22) [$!?] ➜ ag "Alt" ara.after 21:# 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
On Tue, 27 Jun 2023 at 10:57, Alex Bennée <alex.bennee@linaro.org> wrote: > Ahh in the other thread. No change, it looks like AltGr just doesn't > exist for some keymaps: > > 🕙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 > 🕙21:20:50 alex@zen:qemu.git/builds/all (399fc0c) (REBASING 2/22) [$!?] > ➜ ag "Alt" ara.after > 21:# 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 I'm having some difficulty interpreting this output. It seems to show that there is an AltGr modifier in both mappings (that's why it appears in the modifier listing). And for me (xkeyboard-config 2.33) in the gb mapping it's used too: # evdev 2 (0x2), QKeyCode "1", number 0x2 1 0x02 exclam 0x02 shift onesuperior 0x02 altgr exclamdown 0x02 shift altgr (i.e. the '1' key is 1 with no modifiers, ! with shift, superscript-1 with altgr, and inverted exclamation mark with shift-altgr). The 'ara' keymap likewise has and uses altgr: # evdev 2 (0x2), QKeyCode "1", number 0x2 1 0x02 exclam 0x02 shift Arabic_1 0x02 altgr So on the machines where we were running into this, what's the version of xkeyboard-config and do we output the same mapping as we do on machines with the older xkeyboard-config ? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 27 Jun 2023 at 10:57, Alex Bennée <alex.bennee@linaro.org> wrote: >> Ahh in the other thread. No change, it looks like AltGr just doesn't >> exist for some keymaps: >> >> 🕙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 >> 🕙21:20:50 alex@zen:qemu.git/builds/all (399fc0c) (REBASING 2/22) [$!?] >> ➜ ag "Alt" ara.after >> 21:# 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 > > I'm having some difficulty interpreting this output. It > seems to show that there is an AltGr modifier in both > mappings (that's why it appears in the modifier listing). > And for me (xkeyboard-config 2.33) in the gb mapping it's > used too: > > # evdev 2 (0x2), QKeyCode "1", number 0x2 > 1 0x02 > exclam 0x02 shift > onesuperior 0x02 altgr > exclamdown 0x02 shift altgr > > (i.e. the '1' key is 1 with no modifiers, ! with shift, > superscript-1 with altgr, and inverted exclamation mark > with shift-altgr). > > The 'ara' keymap likewise has and uses altgr: > # evdev 2 (0x2), QKeyCode "1", number 0x2 > 1 0x02 > exclam 0x02 shift > Arabic_1 0x02 altgr Ahh right I see those too. > > So on the machines where we were running into this, > what's the version of xkeyboard-config and do we > output the same mapping as we do on machines with > the older xkeyboard-config ? 2.35 I think: ii xkb-data 2.35.1-1 all X Keyboard Extension (XKB) configuration data > > thanks > -- PMM
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);
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(-)