Re: [PATCH v14 01/10] eficonfig: menu-driven addition of UEFI boot option

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

On Tue, 30 Aug 2022 at 14:53, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Kojima-san,
>
> [...]
> > +/**
> > + * eficonfig_choice_entry() - user key input handler
> > + *
> > + * @data:    pointer to the efimenu structure
> > + * Return:   key string to identify the selected entry
> > + */
> > +static char *eficonfig_choice_entry(void *data)
> > +{
> > +     int esc = 0;
> > +     struct list_head *pos, *n;
> > +     struct eficonfig_entry *entry;
> > +     enum bootmenu_key key = KEY_NONE;
> > +     struct efimenu *efi_menu = data;
> > +
> > +     while (1) {
> > +             bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
> > +
> > +             switch (key) {
> > +             case KEY_UP:
> > +                     if (efi_menu->active > 0)
> > +                             --efi_menu->active;
> > +                     /* no menu key selected, regenerate menu */
> > +                     return NULL;
> > +             case KEY_DOWN:
> > +                     if (efi_menu->active < efi_menu->count - 1)
> > +                             ++efi_menu->active;
> > +                     /* no menu key selected, regenerate menu */
> > +                     return NULL;
> > +             case KEY_SELECT:
> > +                     list_for_each_safe(pos, n, &efi_menu->list) {
> > +                             entry = list_entry(pos, struct eficonfig_entry, list);
> > +                             if (entry->num == efi_menu->active)
> > +                                     return entry->key;
> > +                     }
> > +                     break;
> > +             case KEY_QUIT:
> > +                     /* Quit by choosing the last entry */
> > +                     entry = list_last_entry(&efi_menu->list, struct eficonfig_entry, list);
> > +                     return entry->key;
> > +             default:
> > +                     /* Pressed key is not valid, no need to regenerate the menu */
>
> Even with this comment I am still not sure I am following.
> The function definition is a char *.  There are cases ('default' and
> 'KEY_SELECT') which have no return value at all.
> Shouldn't there be a 'return NULL' in the end of the function, even if
> that's not supposed to happen?

nvm, I missed the while(1) loop at the beginning.  This is fine

Cheers
/Ilias
>
> [...]
>
> Thanks
> /Ilias
>