[PATCH v2 2/3] eficonfig: refactor change boot order implementation

[Masahisa Kojima]

On Wed, 28 Dec 2022 at 00:05, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Sat, Dec 24, 2022 at 07:57:43AM +0900, Masahisa Kojima wrote:
> > This commit removes the change boot order specific
> > menu implementation. The change boot order implementation
> > calls eficonfig_process_common() same as other menus.
> >
> > The change boot order menu requires own item_data_print
> > and item_choice implementation, but display_statusline
> > function can be a same function as other menus.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> > Changes in v2:
> > - add comment when the user key press is not valid
> > - add const qualifier to eficonfig_change_boot_order_desc
> >
> >  cmd/eficonfig.c | 245 +++++++++++++++++++++++++++++-------------------
> >  1 file changed, 150 insertions(+), 95 deletions(-)
> >
> > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > index 2fc486dac2..13929cb003 100644
> > --- a/cmd/eficonfig.c
> > +++ b/cmd/eficonfig.c
> > @@ -24,6 +24,11 @@ static struct efi_simple_text_input_protocol *cin;
> >  const char *eficonfig_menu_desc =
> >       "  Press UP/DOWN to move, ENTER to select, ESC/CTRL+C to quit";
> >
> > +static const char *eficonfig_change_boot_order_desc =
> > +     "  Press UP/DOWN to move, +/- to change orde\n"
> > +     "  Press SPACE to activate or deactivate the entry\n"
> > +     "  Select [Save] to complete, ESC/CTRL+C to quit";
> > +
> >  #define EFICONFIG_DESCRIPTION_MAX 32
> >  #define EFICONFIG_OPTIONAL_DATA_MAX 64
> >
> > @@ -105,6 +110,17 @@ struct eficonfig_boot_order_data {
> >       bool active;
> >  };
> >
> > +/**
> > + * struct eficonfig_save_boot_order_data - structure to be used to change boot order
> > + *
> > + * @efi_menu:                pointer to efimenu structure
> > + * @selected:                flag to indicate user selects "Save" entry
> > + */
> > +struct eficonfig_save_boot_order_data {
> > +     struct efimenu *efi_menu;
> > +     bool selected;
> > +};
> > +
> >  /**
> >   * eficonfig_print_msg() - print message
> >   *
> > @@ -173,10 +189,9 @@ void eficonfig_display_statusline(struct menu *m)
> >             "\n%s\n"
> >              ANSI_CURSOR_POSITION ANSI_CLEAR_LINE ANSI_CURSOR_POSITION
> >              "%s"
> > -            ANSI_CLEAR_LINE_TO_END ANSI_CURSOR_POSITION ANSI_CLEAR_LINE,
> > +            ANSI_CLEAR_LINE_TO_END,
> >              1, 1, entry->efi_menu->menu_header, entry->efi_menu->count + 5, 1,
> > -            entry->efi_menu->count + 6, 1, entry->efi_menu->menu_desc,
> > -            entry->efi_menu->count + 7, 1);
> > +            entry->efi_menu->count + 6, 1, entry->efi_menu->menu_desc);
> >  }
> >
> >  /**
> > @@ -1841,63 +1856,44 @@ out:
> >  }
> >
> >  /**
> > - * eficonfig_display_change_boot_order() - display the BootOrder list
> > + * eficonfig_print_change_boot_order_entry() - print the boot option entry
> >   *
> > - * @efi_menu:        pointer to the efimenu structure
> > - * Return:   status code
> > + * @data:    pointer to the data associated with each menu entry
> >   */
> > -static void eficonfig_display_change_boot_order(struct efimenu *efi_menu)
> > +static void eficonfig_print_change_boot_order_entry(void *data)
> >  {
> > -     bool reverse;
> > -     struct list_head *pos, *n;
> > -     struct eficonfig_entry *entry;
> > -
> > -     printf(ANSI_CLEAR_CONSOLE ANSI_CURSOR_POSITION
> > -            "\n  ** Change Boot Order **\n"
> > -            ANSI_CURSOR_POSITION
> > -            "  Press UP/DOWN to move, +/- to change order"
> > -            ANSI_CURSOR_POSITION
> > -            "  Press SPACE to activate or deactivate the entry"
> > -            ANSI_CURSOR_POSITION
> > -            "  Select [Save] to complete, ESC/CTRL+C to quit"
> > -            ANSI_CURSOR_POSITION ANSI_CLEAR_LINE,
> > -            1, 1, efi_menu->count + 5, 1, efi_menu->count + 6, 1,
> > -            efi_menu->count + 7, 1,  efi_menu->count + 8, 1);
> > -
> > -     /* draw boot option list */
> > -     list_for_each_safe(pos, n, &efi_menu->list) {
> > -             entry = list_entry(pos, struct eficonfig_entry, list);
> > -             reverse = (entry->num == efi_menu->active);
> > +     struct eficonfig_entry *entry = data;
> > +     int reverse = (entry->efi_menu->active == entry->num);
>
> Why did we change this from bool?

This should be bool. I will fix it.

Thanks,
Masahisa Kojima

>
> [...]
>
> > -             printf(ANSI_CURSOR_POSITION, entry->num + 4, 7);
> >  }
> >
> >  /**
> > - * eficonfig_choice_change_boot_order() - handle the BootOrder update
> > + * eficonfig_choice_change_boot_order() - user key input handler
> >   *
> > - * @efi_menu:        pointer to the efimenu structure
> > - * Return:   status code
> > + * @data:    pointer to the menu entry
> > + * Return:   key string to identify the selected entry
> >   */
> > -static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
> >  }
> >
> > +/**
> > + * eficonfig_process_save_boot_order() - callback function for "Save" entry
> > + *
> > + * @data:    pointer to the data
> > + * Return:   status code
> > + */
> > +static efi_status_t eficonfig_process_save_boot_order(void *data)
> > +{
> > +     u32 count = 0;
> > +     efi_status_t ret;
> > +     efi_uintn_t size;
> > +     struct list_head *pos, *n;
> > +     u16 *new_bootorder;
> > +     struct efimenu *efi_menu;
> > +     struct eficonfig_entry *entry;
> > +     struct eficonfig_save_boot_order_data *save_data = data;
> > +
> > +     efi_menu = save_data->efi_menu;
> > +
> > +     /*
> > +      * The change boot order menu always has "Save" and "Quit" entries.
> [...]
>
>
> Acked-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>