[PATCH 1/2] menu: Make some parts of the menu available to other components

[Schrempf Frieder]

On 05.02.20 14:44, Frieder Schrempf wrote:
> Hi Simon,
> 
> On 30.12.19 02:21, Simon Glass wrote:
>> Hi Schrempf,
>>
>> On Tue, 10 Dec 2019 at 08:47, Schrempf Frieder
>> <frieder.schrempf at kontron.de> wrote:
>>>
>>> From: Frieder Schrempf <frieder.schrempf at kontron.de>
>>>
>>> In order to iterate over the menu entries and match for a specific
>>> name in the pxe boot, we need to properly export the needed structs
>>> and functions.
>>>
>>> Signed-off-by: Frieder Schrempf <frieder.schrempf at kontron.de>
>>> ---
>>>   common/menu.c  | 31 +------------------------------
>>> Â  include/menu.h | 34 +++++++++++++++++++++++++++++++++-
>>> Â  2 files changed, 34 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/common/menu.c b/common/menu.c
>>> index 7b66d199a9..82b03f17f7 100644
>>> --- a/common/menu.c
>>> +++ b/common/menu.c
>>> @@ -12,36 +12,7 @@
>>>
>>> Â  #include "menu.h"
>>>
>>> -/*
>>> - * Internally, each item in a menu is represented by a struct 
>>> menu_item.
>>> - *
>>> - * These items will be alloc'd and initialized by menu_item_add and 
>>> destroyed
>>> - * by menu_item_destroy, and the consumer of the interface never 
>>> sees that
>>> - * this struct is used at all.
>>> - */
>>> -struct menu_item {
>>> -Â Â Â Â Â Â  char *key;
>>> -Â Â Â Â Â Â  void *data;
>>> -Â Â Â Â Â Â  struct list_head list;
>>> -};
>>>
>>> -/*
>>> - * The menu is composed of a list of items along with settings and 
>>> callbacks
>>> - * provided by the user. An incomplete definition of this struct is 
>>> available
>>> - * in menu.h, but the full definition is here to prevent consumers from
>>> - * relying on its contents.
>>> - */
>>> -struct menu {
>>> -Â Â Â Â Â Â  struct menu_item *default_item;
>>> -Â Â Â Â Â Â  int timeout;
>>> -Â Â Â Â Â Â  char *title;
>>> -Â Â Â Â Â Â  int prompt;
>>> -Â Â Â Â Â Â  void (*item_data_print)(void *);
>>> -Â Â Â Â Â Â  char *(*item_choice)(void *);
>>> -Â Â Â Â Â Â  void *item_choice_data;
>>> -Â Â Â Â Â Â  struct list_head items;
>>> -Â Â Â Â Â Â  int item_cnt;
>>> -};
>>>
>>> Â  /*
>>> Â Â  * An iterator function for menu items. callback will be called for 
>>> each item
>>> @@ -51,7 +22,7 @@ struct menu {
>>> Â Â  * used for search type operations. It is also safe for callback to 
>>> remove the
>>> Â Â  * item from the list of items.
>>> Â Â  */
>>> -static inline void *menu_items_iter(struct menu *m,
>>> +inline void *menu_items_iter(struct menu *m,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  void *(*callback)(struct menu *, struct menu_item *, 
>>> void *),
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  void *extra)
>>> Â  {
>>> diff --git a/include/menu.h b/include/menu.h
>>> index 2d227c20bd..b3f8db87e4 100644
>>> --- a/include/menu.h
>>> +++ b/include/menu.h
>>> @@ -6,8 +6,40 @@
>>> Â  #ifndef __MENU_H__
>>> Â  #define __MENU_H__
>>>
>>> -struct menu;
>>> +/*
>>> + * Internally, each item in a menu is represented by a struct 
>>> menu_item.
>>> + *
>>> + * These items will be alloc'd and initialized by menu_item_add and 
>>> destroyed
>>> + * by menu_item_destroy, and the consumer of the interface never 
>>> sees that
>>> + * this struct is used at all.
>>> + */
>>> +struct menu_item {
>>> +Â Â Â Â Â Â  char *key;
>>> +Â Â Â Â Â Â  void *data;
>>> +Â Â Â Â Â Â  struct list_head list;
>>> +};
>>> +
>>> +/*
>>> + * The menu is composed of a list of items along with settings and 
>>> callbacks
>>> + * provided by the user. An incomplete definition of this struct is 
>>> available
>>> + * in menu.h, but the full definition is here to prevent consumers from
>>> + * relying on its contents.
>>
>> This comment doesn't seem to be true anymore.
> 
> Right.
> 
>>
>> Is it possible to add a function to get the info you need from the menu?
> 
> Unfortunately I didn't find a better solution, that doesn't expose the 
> menu structures. The problem is that menu and pxe are not separated 
> cleanly as the menu items contain references to struct pxe_label.

Actually, on second thought, if I don't use the existing iterator 
function in the menu code, I could keep all the menu code in menu.c and 
expose just one additional function.

I'll send a v2 with this approach.

> 
> If I would add a function to iterate over the items to the menu code, I 
> would need to make struct pxe_label known to the menu code, which 
> doesn't seem nice, either.
> 
> Please let me know if you propose a different approach.
> 
> Thanks,
> Frieder
> 
>>
>>> + */
>>> +struct menu {
>>> +Â Â Â Â Â Â  struct menu_item *default_item;
>>> +Â Â Â Â Â Â  int timeout;
>>> +Â Â Â Â Â Â  char *title;
>>> +Â Â Â Â Â Â  int prompt;
>>> +Â Â Â Â Â Â  void (*item_data_print)(void *);
>>> +Â Â Â Â Â Â  char *(*item_choice)(void *);
>>> +Â Â Â Â Â Â  void *item_choice_data;
>>> +Â Â Â Â Â Â  struct list_head items;
>>> +Â Â Â Â Â Â  int item_cnt;
>>> +};
>>>
>>> +void *menu_items_iter(struct menu *m,
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â  void *(*callback)(struct menu *, struct menu_item *, 
>>> void *),
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â  void *extra);
>>> Â  struct menu *menu_create(char *title, int timeout, int prompt,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  void (*item_data_print)(void *),
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  char *(*item_choice)(void *),
>>> -- 
>>> 2.17.1
>>
>> Regards,
>> Simon
>>