diff mbox

[U-Boot,v2,4/6] Add generic, reusable menu code

Message ID 1308853655-12407-5-git-send-email-jason.hobbs@calxeda.com
State Superseded
Headers show

Commit Message

Jason Hobbs June 23, 2011, 6:27 p.m. UTC
This will be used first by the pxecfg code, but is intended to be
generic and reusable for other jobs in U-boot.

Signed-off-by: Jason Hobbs <jason.hobbs@calxeda.com>
---
changes in v2:
- new in v2

 common/Makefile |    1 +
 common/menu.c   |  338 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/menu.h  |   40 +++++++
 3 files changed, 379 insertions(+), 0 deletions(-)
 create mode 100644 common/menu.c
 create mode 100644 include/menu.h

Comments

Wolfgang Denk June 23, 2011, 8:03 p.m. UTC | #1
Dear "Jason Hobbs",

In message <1308853655-12407-5-git-send-email-jason.hobbs@calxeda.com> you wrote:
> This will be used first by the pxecfg code, but is intended to be
> generic and reusable for other jobs in U-boot.
> 
> Signed-off-by: Jason Hobbs <jason.hobbs@calxeda.com>
> ---
> changes in v2:
> - new in v2
> 
>  common/Makefile |    1 +
>  common/menu.c   |  338 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/menu.h  |   40 +++++++
>  3 files changed, 379 insertions(+), 0 deletions(-)
>  create mode 100644 common/menu.c
>  create mode 100644 include/menu.h

Is this all new code, or was it copied from some other project?  If so,
reference would be needed.

Then, we need documentation how this is going to be used. Please add a
README with documentation of the interfaces and usage examples.

Also, I would like to know why you didn't adapt for example the menu
code from barebox - which specific advantages do you see in the code
you have chosen?

Best regards,

Wolfgang Denk
Jason Hobbs June 24, 2011, 8:25 p.m. UTC | #2
On Thu, Jun 23, 2011 at 10:03:12PM +0200, Wolfgang Denk wrote:
> Dear "Jason Hobbs",
> Is this all new code, or was it copied from some other project?  If
> so, reference would be needed.

All new code.

> Then, we need documentation how this is going to be used. Please add a
> README with documentation of the interfaces and usage examples.

Ok - I'll add this.

> Also, I would like to know why you didn't adapt for example the menu
> code from barebox - which specific advantages do you see in the code
> you have chosen?

I looked over barebox's menu code after you pointed it out, and
it does have more functionality and a fancier interface than this
implementation. Unfortunately, it appears that there aren't any menus
defined to use it in the barebox codebase to provide a real example for
it.

The primary advantage of the implementation I've provided over the
barebox implementation is simplicity.

From a user interface perspective, there are no ANSI terminal codes
used, and selection of a choice is just typing out the name of the
desired choice. It should work on any terminal, and would be easy to
use with expect type automation tools. This simple prompt is also the
default behavior for pxelinux, so should be familiar to end users.

From an implementation perspective, the code does exactly what's needed
for pxecfg without adding extra features that would require some
invention of functionality to prevent introduction of untested and dead
code. It could work equally well for other jobs that just require a
user to make a choice between some options presented on the screen, and
if anything else is needed in the future, it should be extensible to
accommodate. The simplicity makes the code size small - ~1k when built
for my platform, vs ~2.5k for barebox's menu code.

An example complexity from barebox's menu code we don't use here is each
menu entry having its own display, destroy, and "call when selected
action" method. Instead, we implement display the same way for all
menus, destroy the same way for all entries in a menu, and the selected
entry is merely returned to the caller for it to handle.

The implementation also hides the details of the menu's internal
structure from consumers of the interface - the menu and menu_item
struct's are not externally defined, forcing all interaction to go
through the function interfaces. Interface consumers provide a name and
an opaque pointer to a data structure of the user's choice for each
item, and the rest is shielded from the consumer's view. This should
help prevent coding errors and make maintenance of the internals easier
by preventing consumers from relying the implementation rather than the
interface.

Thanks,
Jason
Wolfgang Denk June 24, 2011, 8:45 p.m. UTC | #3
Dear "Jason Hobbs",

In message <20110624202511.GA18667@jhobbs-laptop> you wrote:
> On Thu, Jun 23, 2011 at 10:03:12PM +0200, Wolfgang Denk wrote:
> > Dear "Jason Hobbs",
> > Is this all new code, or was it copied from some other project?  If
> > so, reference would be needed.
> 
> All new code.
> 
> > Then, we need documentation how this is going to be used. Please add a
> > README with documentation of the interfaces and usage examples.
> 
> Ok - I'll add this.
> 
> > Also, I would like to know why you didn't adapt for example the menu
> > code from barebox - which specific advantages do you see in the code
> > you have chosen?
> 
> I looked over barebox's menu code after you pointed it out, and
> it does have more functionality and a fancier interface than this
> implementation. Unfortunately, it appears that there aren't any menus
> defined to use it in the barebox codebase to provide a real example for
> it.
> 
> The primary advantage of the implementation I've provided over the
> barebox implementation is simplicity.
...

Thanks for the detailed explanation.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/common/Makefile b/common/Makefile
index f81cff9..9b413cd 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -171,6 +171,7 @@  COBJS-$(CONFIG_CMD_KGDB) += kgdb.o kgdb_stubs.o
 COBJS-$(CONFIG_KALLSYMS) += kallsyms.o
 COBJS-$(CONFIG_LCD) += lcd.o
 COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o
+COBJS-$(CONFIG_MENU) += menu.o
 COBJS-$(CONFIG_MODEM_SUPPORT) += modem.o
 COBJS-$(CONFIG_UPDATE_TFTP) += update.o
 COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
diff --git a/common/menu.c b/common/menu.c
new file mode 100644
index 0000000..e92d302
--- /dev/null
+++ b/common/menu.c
@@ -0,0 +1,338 @@ 
+/*
+ * Copyright 2010-2011 Calxeda, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <errno.h>
+#include <linux/list.h>
+
+#include "menu.h"
+
+struct menu_item {
+	char *key;
+	void *data;
+	struct list_head list;
+};
+
+struct menu {
+	struct menu_item *default_item;
+	int timeout;
+	char *title;
+	int prompt;
+	void (*item_data_print)(void *);
+	void (*item_data_destroy)(void *);
+	struct list_head items;
+};
+
+struct callback_with_data {
+	void *(*callback)(void *, void *);
+	void *data;
+};
+
+static inline void *menu_items_iter(struct menu *m,
+		void *(*callback)(struct menu *, struct menu_item *, void *),
+		void *extra)
+{
+	struct list_head *pos, *n;
+	struct menu_item *item;
+	void *ret;
+
+	list_for_each_safe(pos, n, &m->items) {
+		item = list_entry(pos, struct menu_item, list);
+
+		ret = callback(m, item, extra);
+
+		if (ret)
+			return ret;
+	}
+
+	return NULL;
+}
+
+static inline void *menu_item_print(struct menu *m,
+				struct menu_item *item,
+				void *extra)
+{
+	if (!m->item_data_print)
+		printf("%s:\n", item->key);
+	else
+		m->item_data_print(item->data);
+
+	return NULL;
+}
+
+static inline void *menu_item_destroy(struct menu *m,
+				struct menu_item *item,
+				void *extra)
+{
+	if (m->item_data_destroy)
+		m->item_data_destroy(item->data);
+
+	if (item->key)
+		free(item->key);
+
+	free(item);
+
+	return NULL;
+}
+
+static inline void menu_display(struct menu *m)
+{
+	if (m->title)
+		printf("%s\n", m->title);
+
+	menu_items_iter(m, menu_item_print, NULL);
+}
+
+static inline void *menu_item_key_match(struct menu *m,
+				struct menu_item *item,
+				void *key)
+{
+	if (!key || !item->key) {
+		if (key == item)
+			return item;
+
+		return NULL;
+	}
+
+	if (strcmp(item->key, key) == 0)
+		return item;
+
+	return NULL;
+}
+
+static inline struct menu_item *menu_item_by_key(struct menu *m, char *key)
+{
+	return menu_items_iter(m, menu_item_key_match, key);
+}
+
+static inline void *menu_item_data_cb(struct menu *m,
+				struct menu_item *item,
+				void *extra)
+{
+
+	struct callback_with_data *cb_data = extra;
+
+	return cb_data->callback(item->data, cb_data->data);
+}
+
+static inline int delay_for_input(int timeout)
+{
+	if (!timeout)
+		return 0;
+
+	if (abortboot(timeout/10)) {
+		printf("autoboot aborted!\n");
+		return 1;
+	}
+
+	return 0;
+}
+
+void *menu_items_data_iter(struct menu *m,
+			void *(*callback)(void *, void *),
+			void *extra)
+{
+	struct callback_with_data cb_data;
+
+	if (!m || !callback)
+		return NULL;
+
+	cb_data.callback = callback;
+	cb_data.data = extra;
+
+	return menu_items_iter(m, menu_item_data_cb, &cb_data);
+}
+
+int menu_should_prompt(struct menu *m)
+{
+	if (!m)
+		return -EINVAL;
+
+	return m->prompt || delay_for_input(m->timeout);
+}
+
+int menu_default_set(struct menu *m, char *key)
+{
+	struct menu_item *item;
+
+	if (!m)
+		return -EINVAL;
+
+	item = menu_item_by_key(m, key);
+
+	if (!item)
+		return -ENOENT;
+
+	m->default_item = item;
+
+	return 1;
+}
+
+void *menu_default_choice(struct menu *m)
+{
+	if (!m)
+		return NULL;
+
+	if (m->default_item)
+		return m->default_item->data;
+
+	return NULL;
+}
+
+void *menu_user_choice(struct menu *m)
+{
+	char cbuf[CONFIG_SYS_CBSIZE];
+	struct menu_item *choice_item = NULL;
+
+	if (!m)
+		return NULL;
+
+	while (!choice_item) {
+		int readret;
+
+		cbuf[0] = '\0';
+
+		menu_display(m);
+
+		readret = readline_into_buffer("Enter choice: ", cbuf);
+
+		if (readret >= 0) {
+			choice_item = menu_item_by_key(m, cbuf);
+
+			if (!choice_item)
+				printf("%s not found\n", cbuf);
+			else
+				return choice_item->data;
+		} else {
+			printf("^C\n");
+			return NULL;
+		}
+	}
+
+	return NULL;
+}
+
+/*
+ * note that this replaces the data of an item if it already exists, but
+ * doesn't change the order of the item.
+ */
+int menu_item_add(struct menu *m, char *item_key, void *item_data)
+{
+	struct menu_item *item;
+
+	if (!m)
+		return -EINVAL;
+
+	item = menu_item_by_key(m, item_key);
+
+	if (item) {
+		m->item_data_destroy(item->data);
+		item->data = item_data;
+		return 1;
+	}
+
+	item = malloc(sizeof *item);
+
+	if (!item)
+		return -ENOMEM;
+
+	item->key = strdup(item_key);
+
+	if (!item->key) {
+		free(item);
+		return -ENOMEM;
+	}
+
+	item->data = item_data;
+
+	list_add_tail(&item->list, &m->items);
+
+	return 1;
+}
+
+struct menu *menu_create(void (*item_data_print)(void *),
+			void (*item_data_destroy)(void *))
+{
+	struct menu *m;
+
+	m = malloc(sizeof *m);
+
+	if (!m)
+		return NULL;
+
+	m->default_item = NULL;
+	m->timeout = 0;
+	m->title = NULL;
+	m->prompt = 0;
+
+	m->item_data_print = item_data_print;
+	m->item_data_destroy = item_data_destroy;
+
+	INIT_LIST_HEAD(&m->items);
+
+	return m;
+}
+
+int menu_title_set(struct menu *m, char *title)
+{
+	char *new_title;
+
+	if (!m || !title)
+		return -EINVAL;
+
+	new_title = strdup(title);
+
+	if (!new_title)
+		return -ENOMEM;
+
+	if (m->title)
+		free(m->title);
+
+	m->title = new_title;
+
+	return 1;
+}
+
+void menu_destroy(struct menu *m)
+{
+	if (!m)
+		return;
+
+	menu_items_iter(m, menu_item_destroy, NULL);
+
+	if (m->title)
+		free(m->title);
+
+	free(m);
+}
+
+void menu_prompt_set(struct menu *m, int prompt)
+{
+	if (!m)
+		return;
+
+	m->prompt = prompt;
+}
+
+void menu_timeout_set(struct menu *m, int timeout)
+{
+	if (!m)
+		return;
+
+	m->timeout = timeout;
+}
diff --git a/include/menu.h b/include/menu.h
new file mode 100644
index 0000000..5ffb3b8
--- /dev/null
+++ b/include/menu.h
@@ -0,0 +1,40 @@ 
+/*
+ * Copyright 2010-2011 Calxeda, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __MENU_H__
+#define __MENU_H__
+
+struct menu;
+struct menu_item;
+
+void *menu_items_data_iter(struct menu *m,
+		void *(*callback)(void *, void *),
+		void *extra);
+
+int menu_should_prompt(struct menu *m);
+int menu_default_set(struct menu *m, char *key);
+void *menu_default_choice(struct menu *m);
+void *menu_user_choice(struct menu *m);
+int menu_item_add(struct menu *m, char *item_key, void *item_data);
+struct menu *menu_create(void (*item_data_print)(void *),
+			void (*item_data_destroy)(void *));
+int menu_title_set(struct menu *m, char *title);
+void menu_destroy(struct menu *m);
+void menu_prompt_set(struct menu *m, int prompt);
+void menu_timeout_set(struct menu *m, int timeout);
+
+#endif /* __MENU_H__ */