Re: [Qemu-devel] [RFC PATCH v2 2/7] kbd-state: use state tracker for sdl2

[Daniel P . Berrangé]

On Wed, Dec 19, 2018 at 01:08:59PM +0100, Gerd Hoffmann wrote:
> Use the new keyboard state tracked for sdl2.  We can drop the modifier
> state tracking from sdl2.  Also keyup code is simpler, the state tracker
> will take care to not send suspious keyup events to the guest.
> 
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  include/ui/sdl2.h |  2 ++
>  ui/sdl2-input.c   | 40 +++-------------------------------------
>  ui/sdl2.c         | 12 +++---------
>  3 files changed, 8 insertions(+), 46 deletions(-)
> 
> diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
> index f43eecdbd6..393f4b0fd5 100644
> --- a/include/ui/sdl2.h
> +++ b/include/ui/sdl2.h
> @@ -7,6 +7,7 @@

> diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
> index 1378b63dd9..ae52d548f6 100644
> --- a/ui/sdl2-input.c
> +++ b/ui/sdl2-input.c
> @@ -30,22 +30,9 @@
>  #include "ui/sdl2.h"
>  #include "sysemu/sysemu.h"
>  
> -static uint8_t modifiers_state[SDL_NUM_SCANCODES];
> -
>  void sdl2_reset_keys(struct sdl2_console *scon)
>  {
> -    QemuConsole *con = scon ? scon->dcl.con : NULL;
> -    int i;
> -
> -    for (i = 0 ;
> -         i < SDL_NUM_SCANCODES && i < qemu_input_map_usb_to_qcode_len ;
> -         i++) {
> -        if (modifiers_state[i]) {
> -            int qcode = qemu_input_map_usb_to_qcode[i];
> -            qemu_input_event_send_key_qcode(con, qcode, false);
> -            modifiers_state[i] = 0;
> -        }
> -    }
> +    kbd_state_lift_all_keys(scon->kbd);
>  }

I was wondering if we could just make the callers of sdl2_reset_keys
call  kbd_state_lift_all_keys directly. I can't find any callers of
sdl2_reset_keys at all though. It appears you removed all callers
in

  commit f8d2c9369b8302f65f4f43f14ed3987c2268a02a
  Author: Gerd Hoffmann <address@hidden>
  Date:   Mon Jan 15 16:48:54 2018 +0100

    sdl: use ctrl-alt-g as grab hotkey

So I guess we could have a prior patch that just deletes this method
entirely.


For the rest of the patch

  Reviewed-by: Daniel P. Berrangé <address@hidden>


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 :|