Re: [PATCH v2 3/6] efi/console: Add grub_console_read_key_stroke() helper function

[Daniel Kiper]

On Tue, Apr 07, 2020 at 11:03:48AM +0200, Javier Martinez Canillas wrote:
> From: Hans de Goede <address@hidden>
>
> This is a preparatory patch for adding getkeystatus() support to the
> EFI console driver.
>
> We can get modifier status through the simple_text_input read_key_stroke
> method, but if a non-modifier key is (also) pressed the read_key_stroke
> call will consume that key from the firmware's queue.
>
> The new grub_console_read_key_stroke() helper buffers upto 1 key-stroke.
> If it has a non-modifier key buffered, it will return that one, if its
> buffer is empty, it will fills its buffer by getting a new key-stroke.
>
> If called with consume=1 it will empty its buffer after copying the
> key-data to the callers buffer, this is how getkey() will use it.
>
> If called with consume=0 it will keep the last key-stroke buffered, this
> is how getkeystatus() will call it. This means that if a non-modifier
> key gets pressed, repeated getkeystatus() calls will return the modifiers
> of that key-press until it is consumed by a getkey() call.
>
> Signed-off-by: Hans de Goede <address@hidden>
> Signed-off-by: Javier Martinez Canillas <address@hidden>
> ---
>
> Changes in v2: None
>
>  grub-core/term/efi/console.c | 51 ++++++++++++++++++++++++++++--------
>  1 file changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
> index 59d9604472a..2145f33c32a 100644
> --- a/grub-core/term/efi/console.c
> +++ b/grub-core/term/efi/console.c
> @@ -193,27 +193,56 @@ grub_console_getkey_con (struct grub_term_input *term 
> __attribute__ ((unused)))
>    return grub_efi_translate_key(key);
>  }
>
> +/*
> + * When more then just modifiers are pressed, our getkeystatus() consumes a
> + * press from the queue, this function buffers the press for the regular
> + * getkey() so that it does not get lost.
> + */
>  static int
> -grub_console_getkey_ex(struct grub_term_input *term)
> +grub_console_read_key_stroke (
> +                   grub_efi_simple_text_input_ex_interface_t *text_input,
> +                   grub_efi_key_data_t *key_data_ret, int *key_ret,
> +                   int consume)
>  {
> -  grub_efi_key_data_t key_data;
> +  static grub_efi_key_data_t key_data;
>    grub_efi_status_t status;
> -  grub_efi_uint32_t kss;
> -  int key = -1;
> +  int key;
>
> -  grub_efi_simple_text_input_ex_interface_t *text_input = term->data;
> +  if (!text_input)
> +    return GRUB_ERR_EOF;
>
> -  status = efi_call_2 (text_input->read_key_stroke, text_input, &key_data);
> +  key = grub_efi_translate_key (key_data.key);
> +  if (key == GRUB_TERM_NO_KEY) {
> +    status = efi_call_2 (text_input->read_key_stroke, text_input, &key_data);
> +    if (status != GRUB_EFI_SUCCESS)
> +      return GRUB_ERR_EOF;
>
> -  if (status != GRUB_EFI_SUCCESS)
> -    return GRUB_TERM_NO_KEY;
> +    key = grub_efi_translate_key (key_data.key);
> +  }
>
> -  kss = key_data.key_state.key_shift_state;
> -  key = grub_efi_translate_key(key_data.key);
> +  *key_data_ret = key_data;
> +  *key_ret = key;
> +
> +  if (consume) {
> +    key_data.key.scan_code = 0;
> +    key_data.key.unicode_char = 0;
> +  }
> +
> +  return 0;

Nit, return GRUB_ERR_NONE...

> +}
>
> -  if (key == GRUB_TERM_NO_KEY)
> +static int
> +grub_console_getkey_ex (struct grub_term_input *term)
> +{
> +  grub_efi_key_data_t key_data;
> +  grub_efi_uint32_t kss;
> +  int key = -1;
> +
> +  if (grub_console_read_key_stroke (term->data, &key_data, &key, 1) ||

== GRUB_ERR_NONE

Otherwise LGTM...

Daniel