"Hi Ilias, On Sat, 5 Nov 2022 at 00:12, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Kojima-san > > I think there's some information missing from the commit message. > > On Wed, Oct 26, 2022 at 07:43:41PM +0900, Masahisa Kojima wrote: > > eficonfig_select_file_handler() is commonly used to select the > > file. > > eficonfig_display_select_file_option() intends to add the > > additional menu mainly to clear the selected file information. > 'eficonfig_display_select_file_option() adds an additional menu to clear > the selected file' sounds better?
OK, I will update the commit message. > > > eficonfig_display_select_file_option() is not necessary for the > > file selection process, so it should be outside of > > eficonfig_select_file_handler(). > > In a followup patch I think we should rename eficonfig_select_file(). It's > a bit confusing to have both eficonfig_select_file_handler() and > eficonfig_select_file() and iirc the latter creates the menu for the file > selection. Yes, I agree. I will rename the functions as follows. eficonfig_select_file_handler -> eficonfig_process_select_file() eficonfig_select_file -> eficonfig_show_file_selection() > > > > > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org> > > --- > > No change since v2 > > > > newly created in v2 > > > > cmd/eficonfig.c | 13 +++++-------- > > test/py/tests/test_eficonfig/test_eficonfig.py | 1 + > > 2 files changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > > index 2595dd9563..f6a99bd01a 100644 > > --- a/cmd/eficonfig.c > > +++ b/cmd/eficonfig.c > > @@ -968,7 +968,7 @@ efi_status_t > > eficonfig_process_clear_file_selection(void *data) > > } > > > > static struct eficonfig_item select_file_menu_items[] = { > > - {"Select File", eficonfig_process_select_file}, > > eficonfig_process_select_file() is not used anywhere anymore now. We need > to get rid of the function declaration Yes, I will remove it. Note that this function name "eficonfig_process_select_file" will be used for renaming "eficonfig_select_file_handler" in the followup patch. > > > + {"Select File", eficonfig_select_file_handler}, > > This is a different change right? It effectively allows you to choose > between files. > Can we explain this change as well on the commit message? Yes, I will update the commit message. Thank you for your feedback. Regards, Masahisa Kojima > > > {"Clear", eficonfig_process_clear_file_selection}, > > {"Quit", eficonfig_process_quit}, > > }; > > @@ -980,12 +980,13 @@ static struct eficonfig_item select_file_menu_items[] > > = { > > * @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_display_select_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) > > @@ -1016,10 +1017,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; > > @@ -1284,7 +1281,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_display_select_file_option, > > file_info); > > out: > > free(devname); > > free(file_name); > > 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 > > > > Thanks > /Ilias