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

From: Frieder Schrempf <frieder.schrempf@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@kontron.de>
---
 common/menu.c  | 31 +------------------------------
 include/menu.h | 34 +++++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 31 deletions(-)

Hi Schrempf,

On Tue, 10 Dec 2019 at 08:47, Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:
>
> From: Frieder Schrempf <frieder.schrempf@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@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.

Is it possible to add a function to get the info you need from the menu?

> + */
> +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

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@kontron.de> wrote:
>>
>> From: Frieder Schrempf <frieder.schrempf@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@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.

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
>

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@kontron.de> wrote:
>>>
>>> From: Frieder Schrempf <frieder.schrempf@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@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
>>