diff mbox series

[v4,1/2] menu: Add a function to set the default by matching the item data

Message ID 20200212103629.12746-1-frieder.schrempf@kontron.de
State New
Headers show
Series [v4,1/2] menu: Add a function to set the default by matching the item data | expand

Commit Message

Frieder Schrempf Feb. 12, 2020, 10:37 a.m. UTC
From: Frieder Schrempf <frieder.schrempf at 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 at 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(+)

Comments

Frieder Schrempf Feb. 12, 2020, 10:42 a.m. UTC | #1
On 12.02.20 11:37, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf at 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 at 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(+)
> 
> 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)
> +{
> +	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;
> +	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

I just saw, that there's a superfluous hyphen here. Sorry, will fix it 
if a new version is requested.

> + * 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.
> + * @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
>    *
>
Simon Glass Feb. 12, 2020, 5:14 p.m. UTC | #2
On Wed, 12 Feb 2020 at 03:37, Schrempf Frieder
<frieder.schrempf at kontron.de> wrote:
>
> From: Frieder Schrempf <frieder.schrempf at 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 at 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
diff mbox series

Patch

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)
+{
+	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;
+	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.
+ * @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
  *