Re: [PATCH v7 1/5] eficonfig: refactor file selection handling

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

On Wed, Nov 09, 2022 at 12:37:24PM +0900, Masahisa Kojima wrote:
> eficonfig_select_file_handler() is commonly used to select the
> file. eficonfig_display_select_file_option() adds an additional
> menu to clear the selected file.
> eficonfig_display_select_file_option() is not always necessary
> for the file selection process, so it must be outside of
> eficonfig_select_file_handler().
> 
> This commit also renames the following functions to avoid confusion.
>  eficonfig_select_file_handler() -> eficonfig_process_select_file()
>  eficonfig_select_file() -> eficonfig_show_file_selection()
>  eficonfig_display_select_file_option() -> eficonfig_process_show_file_option()
> 
> Finally, test_eficonfig.py need to be updated to get aligned with
> the above modification.
> 
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Changes in v7:
> - rename functio name to avoid confusion
> - remove unused function
> - update commit message
> 
> newly created in v2
> 
>  cmd/eficonfig.c                               | 37 ++++++-------------
>  include/efi_config.h                          |  2 +-
>  .../py/tests/test_eficonfig/test_eficonfig.py |  1 +
>  3 files changed, 13 insertions(+), 27 deletions(-)
> 
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 2595dd9563..571e2b9ac0 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -756,14 +756,14 @@ out:
>  }
>  
>  /**
> - * eficonfig_select_file() - construct the file selection menu
> + * eficonfig_show_file_selection() - construct the file selection menu
>   *
>   * @file_info:	pointer to the file selection structure
>   * @root:	pointer to the file handle
>   * Return:	status code
>   */
> -static efi_status_t eficonfig_select_file(struct eficonfig_select_file_info *file_info,
> -					  struct efi_file_handle *root)
> +static efi_status_t eficonfig_show_file_selection(struct eficonfig_select_file_info *file_info,
> +						  struct efi_file_handle *root)
>  {
>  	u32 count = 0, i;
>  	efi_uintn_t len;
> @@ -938,17 +938,6 @@ static efi_status_t eficonfig_boot_edit_save(void *data)
>  	return EFI_SUCCESS;
>  }
>  
> -/**
> - * eficonfig_process_select_file() - callback function for "Select File" entry
> - *
> - * @data:	pointer to the data
> - * Return:	status code
> - */
> -efi_status_t eficonfig_process_select_file(void *data)
> -{
> -	return EFI_SUCCESS;
> -}
> -
>  /**
>   * eficonfig_process_clear_file_selection() - callback function for "Clear" entry
>   *
> @@ -973,19 +962,19 @@ static struct eficonfig_item select_file_menu_items[] = {
>  	{"Quit", eficonfig_process_quit},
>  };
>  
> -
>  /**
> - * eficonfig_display_select_file_option() - display select file option
> + * eficonfig_process_show_file_option() - display select file option
>   *
>   * @file_info:	pointer to the file information structure
>   * Return:	status code
>   */
> -efi_status_t eficonfig_display_select_file_option(struct eficonfig_select_file_info *file_info)
> +efi_status_t eficonfig_process_show_file_option(void *data)
>  {
>  	efi_status_t ret;
>  	struct efimenu *efi_menu;
>  
> -	select_file_menu_items[1].data = file_info;
> +	select_file_menu_items[0].data = data;
> +	select_file_menu_items[1].data = data;
>  	efi_menu = eficonfig_create_fixed_menu(select_file_menu_items,
>  					       ARRAY_SIZE(select_file_menu_items));
>  	if (!efi_menu)
> @@ -1001,12 +990,12 @@ efi_status_t eficonfig_display_select_file_option(struct eficonfig_select_file_i
>  }
>  
>  /**
> - * eficonfig_select_file_handler() - handle user file selection
> + * eficonfig_process_select_file() - handle user file selection
>   *
>   * @data:	pointer to the data
>   * Return:	status code
>   */
> -efi_status_t eficonfig_select_file_handler(void *data)
> +efi_status_t eficonfig_process_select_file(void *data)
>  {
>  	size_t len;
>  	efi_status_t ret;
> @@ -1016,10 +1005,6 @@ efi_status_t eficonfig_select_file_handler(void *data)
>  	struct eficonfig_select_file_info *tmp = NULL;
>  	struct eficonfig_select_file_info *file_info = data;
>  
> -	ret = eficonfig_display_select_file_option(file_info);
> -	if (ret != EFI_SUCCESS)
> -		return ret;
> -
>  	tmp = calloc(1, sizeof(struct eficonfig_select_file_info));
>  	if (!tmp)
>  		return EFI_OUT_OF_RESOURCES;
> @@ -1046,7 +1031,7 @@ efi_status_t eficonfig_select_file_handler(void *data)
>  		if (ret != EFI_SUCCESS)
>  			goto out;
>  
> -		ret = eficonfig_select_file(tmp, root);
> +		ret = eficonfig_show_file_selection(tmp, root);
>  		if (ret == EFI_ABORTED)
>  			continue;
>  		if (ret != EFI_SUCCESS)
> @@ -1284,7 +1269,7 @@ static efi_status_t prepare_file_selection_entry(struct efimenu *efi_menu, char
>  	utf8_utf16_strcpy(&p, devname);
>  	u16_strlcat(file_name, file_info->current_path, len);
>  	ret = create_boot_option_entry(efi_menu, title, file_name,
> -				       eficonfig_select_file_handler, file_info);
> +				       eficonfig_process_show_file_option, file_info);
>  out:
>  	free(devname);
>  	free(file_name);
> diff --git a/include/efi_config.h b/include/efi_config.h
> index 098cac2115..cc6aa51393 100644
> --- a/include/efi_config.h
> +++ b/include/efi_config.h
> @@ -89,7 +89,7 @@ void eficonfig_print_msg(char *msg);
>  void eficonfig_destroy(struct efimenu *efi_menu);
>  efi_status_t eficonfig_process_quit(void *data);
>  efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_header);
> -efi_status_t eficonfig_select_file_handler(void *data);
> +efi_status_t eficonfig_process_select_file(void *data);
>  efi_status_t eficonfig_get_unused_bootoption(u16 *buf,
>  					     efi_uintn_t buf_size, u32 *index);
>  efi_status_t eficonfig_append_bootorder(u16 index);
> diff --git a/test/py/tests/test_eficonfig/test_eficonfig.py b/test/py/tests/test_eficonfig/test_eficonfig.py
> index 99606d9c4b..102bfd7541 100644
> --- a/test/py/tests/test_eficonfig/test_eficonfig.py
> +++ b/test/py/tests/test_eficonfig/test_eficonfig.py
> @@ -349,6 +349,7 @@ def test_efi_eficonfig(u_boot_console, efi_eficonfig_data):
>          press_up_down_enter_and_wait(0, 1, True, 'Quit')
>          press_up_down_enter_and_wait(0, 0, True, 'No block device found!')
>          press_escape_key(False)
> +        press_escape_key(False)
>          check_current_is_maintenance_menu()
>          # Return to U-Boot console
>          press_escape_key(True)
> -- 
> 2.17.1
> 

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>