On Wed, 12 Feb 2020 at 03:37, Schrempf Frieder frieder.schrempf@kontron.de wrote:
From: Frieder Schrempf frieder.schrempf@kontron.de
In order to make it possible to auto select a default entry by matching the data of the menu entries by an external matching function, we add some helpers and expose the menu_set_default_by_item_data_match() function.
Signed-off-by: Frieder Schrempf frieder.schrempf@kontron.de
Changes in v4:
- Use the proper style for the function description
- Move the comment for the function description to the header file
Changes in v3:
- Add a full function comment to describe menu_set_default_by_item_data_match().
Changes in v2:
- Keep the menu structs private and instead only expose one additional function, that sets the default by calling an external matching function on each entry.
- Change the title and commit message to reflect the changes.
common/menu.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/menu.h | 19 +++++++++++++++++++ 2 files changed, 59 insertions(+)
Looks good.
diff --git a/common/menu.c b/common/menu.c index 7b66d199a9..e16a0a4a50 100644 --- a/common/menu.c +++ b/common/menu.c @@ -160,6 +160,46 @@ static inline struct menu_item *menu_item_by_key(struct menu *m, return menu_items_iter(m, menu_item_key_match, item_key); }
+/*
- Find the first matching item, if any exists by calling a matching function
- on the items data field.
- */
+static inline struct menu_item *menu_item_by_matching_fn(struct menu *m,
int match(void *, void *), void * extra)
No space after *
What is extra? This function needs a comment I think.
+{
struct list_head *pos, *n;struct menu_item *item;int ret;list_for_each_safe(pos, n, &m->items) {item = list_entry(pos, struct menu_item, list);if (item->key) {ret = match(item->data, extra);if (ret == 1)return item;}}return NULL;+}
+/*
- Sets a menu default option by calling a matching function on each of the
- menu items data field.
- */
+int menu_set_default_by_item_data_match(struct menu *m,
int match(void *, void *), void *extra, char **key)+{
struct menu_item *item = menu_item_by_matching_fn(m, match, extra);if (!item)return -ENOENT;*key = item->key;m->default_item = item;
blank line before return
return 0;+}
/*
- Set *choice to point to the default item's data, if any default item was
- set, and returns 1. If no default item was set, returns -ENOENT.
diff --git a/include/menu.h b/include/menu.h index 2d227c20bd..96c9fcac03 100644 --- a/include/menu.h +++ b/include/menu.h @@ -19,6 +19,25 @@ int menu_destroy(struct menu *m); void menu_display_statusline(struct menu *m); int menu_default_choice(struct menu *m, void **choice);
+/**
- menu_set_default_by_item_data_match() - - sets a menu default option by
- calling a matching function on each of the menu items data field.
- @m: Points to a menu created by menu_create().
- @match: Points to a function that is passed a pointer to the items data field
and a pointer to extra data to compare with. It should return 1 on a
match.
else 0 ?
Also, can you document the args to match()? Can they be const *? I actually wonder if you should add a typedef to make this a bit clearer? I know we don't use them for structs but I think they have value with function pointers.
- @extra: Points to some data that is passed as a second parameter to the
matching function.
- @key: Points to a char array that will be set to hold the key of the matched
menu item.
- @return 0 if a matching item was found,
-ENOENT if no matching item was found- */
+int menu_set_default_by_item_data_match(struct menu *m,
int match(void *, void *), void *extra,char **key);/**
- menu_show() Show a boot menu
-- 2.17.1