Re: [PATCH 44/67] ui/vc: change the argument for QemuTextConsole

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Aug 30, 2023 at 01:38:18PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Those functions are specifc to text/vc console, make that explicit from
> the argument type.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/console.h |  6 +++---
>  ui/console.c         | 14 ++++++--------
>  ui/gtk.c             |  2 +-
>  ui/sdl2-input.c      |  5 +++--
>  ui/sdl2.c            |  5 ++---
>  5 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 91d8bbc9dc..1ccd432b4d 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -112,9 +112,9 @@ bool qemu_mouse_set(int index, Error **errp);
>  #define QEMU_KEY_CTRL_PAGEUP     0xe406
>  #define QEMU_KEY_CTRL_PAGEDOWN   0xe407
>  
> -void kbd_put_keysym_console(QemuConsole *s, int keysym);
> -bool kbd_put_qcode_console(QemuConsole *s, int qcode, bool ctrl);
> -void kbd_put_string_console(QemuConsole *s, const char *str, int len);
> +void kbd_put_keysym_console(QemuTextConsole *s, int keysym);
> +bool kbd_put_qcode_console(QemuTextConsole *s, int qcode, bool ctrl);
> +void kbd_put_string_console(QemuTextConsole *s, const char *str, int len);
>  void kbd_put_keysym(int keysym);
>  
>  /* Touch devices */
> diff --git a/ui/console.c b/ui/console.c
> index 9fccecafd7..a98adbb1b2 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1135,16 +1135,12 @@ static void kbd_send_chars(QemuTextConsole *s)
>  }
>  
>  /* called when an ascii key is pressed */
> -void kbd_put_keysym_console(QemuConsole *con, int keysym)
> +void kbd_put_keysym_console(QemuTextConsole *s, int keysym)
>  {
> -    QemuTextConsole *s = (QemuTextConsole *)object_dynamic_cast(OBJECT(con), TYPE_QEMU_TEXT_CONSOLE);
>      uint8_t buf[16], *q;
>      int c;
>      uint32_t num_free;
>  
> -    if (!s)
> -        return;
> -
>      switch(keysym) {
>      case QEMU_KEY_CTRL_UP:
>          console_scroll(s, -1);
> @@ -1214,7 +1210,7 @@ static const int ctrl_qcode_to_keysym[Q_KEY_CODE__MAX] = {
>      [Q_KEY_CODE_PGDN]   = QEMU_KEY_CTRL_PAGEDOWN,
>  };
>  
> -bool kbd_put_qcode_console(QemuConsole *s, int qcode, bool ctrl)
> +bool kbd_put_qcode_console(QemuTextConsole *s, int qcode, bool ctrl)
>  {
>      int keysym;
>  
> @@ -1226,7 +1222,7 @@ bool kbd_put_qcode_console(QemuConsole *s, int qcode, bool ctrl)
>      return true;
>  }
>  
> -void kbd_put_string_console(QemuConsole *s, const char *str, int len)
> +void kbd_put_string_console(QemuTextConsole *s, const char *str, int len)
>  {
>      int i;
>  
> @@ -1237,7 +1233,9 @@ void kbd_put_string_console(QemuConsole *s, const char *str, int len)
>  
>  void kbd_put_keysym(int keysym)
>  {
> -    kbd_put_keysym_console(active_console, keysym);
> +    if (QEMU_IS_TEXT_CONSOLE(active_console)) {
> +        kbd_put_keysym_console(QEMU_TEXT_CONSOLE(active_console), keysym);
> +    }
>  }
>  
>  static void text_console_invalidate(void *opaque)
> diff --git a/ui/gtk.c b/ui/gtk.c
> index ef98bb0648..c34c133550 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1187,7 +1187,7 @@ static gboolean gd_text_key_down(GtkWidget *widget,
>                                   GdkEventKey *key, void *opaque)
>  {
>      VirtualConsole *vc = opaque;
> -    QemuConsole *con = vc->gfx.dcl.con;
> +    QemuTextConsole *con = QEMU_TEXT_CONSOLE(vc->gfx.dcl.con);
>  
>      if (key->keyval == GDK_KEY_Delete) {
>          kbd_put_qcode_console(con, Q_KEY_CODE_DELETE, false);
> diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
> index f068382209..efc0e76e8c 100644
> --- a/ui/sdl2-input.c
> +++ b/ui/sdl2-input.c
> @@ -44,14 +44,15 @@ void sdl2_process_key(struct sdl2_console *scon,
>      qkbd_state_key_event(scon->kbd, qcode, ev->type == SDL_KEYDOWN);
>  
>      if (!qemu_console_is_graphic(con)) {

If i'm nit-picking I'd say change to be

   "if  (QEMU_IS_TEXT_CONSOLE(con)) {"

as you did in the other places in this patch

> +        QemuTextConsole *s = QEMU_TEXT_CONSOLE(con);
>          bool ctrl = qkbd_state_modifier_get(scon->kbd, QKBD_MOD_CTRL);
>          if (ev->type == SDL_KEYDOWN) {
>              switch (qcode) {
>              case Q_KEY_CODE_RET:
> -                kbd_put_keysym_console(con, '\n');
> +                kbd_put_keysym_console(s, '\n');
>                  break;
>              default:
> -                kbd_put_qcode_console(con, qcode, ctrl);
> +                kbd_put_qcode_console(s, qcode, ctrl);
>                  break;
>              }
>          }
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 0d91b555e3..16b515fcf9 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -483,10 +483,9 @@ static void handle_textinput(SDL_Event *ev)
>          return;
>      }
>  
> -    if (qemu_console_is_graphic(con)) {
> -        return;
> +    if (QEMU_IS_TEXT_CONSOLE(con)) {
> +        kbd_put_string_console(QEMU_TEXT_CONSOLE(con), ev->text.text, strlen(ev->text.text));
>      }
> -    kbd_put_string_console(con, ev->text.text, strlen(ev->text.text));
>  }
>  
>  static void handle_mousemotion(SDL_Event *ev)

With or without my suggestion:

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


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