Re: [U-Boot] [PATCH v3 1/7] Add generic, reusable menu code

2011-07-26 Thread Jason Hobbs
Dear Wolfgang,

On Mon, Jul 25, 2011 at 11:18:15PM +0200, Wolfgang Denk wrote:
 I am happy that you provide documentation in doc/README.menu, but I
 really dislike that the code itself is basicly uncommented. It is a
 major pain to have to switch between the README and the source files
 when trying to understand the code.
 
 Please add sufficient comments to the code to make it readable.

I don't want to duplicate the interface documentation in the README and
the source file.  Maybe I'll add shorter summary descriptions to the
README and move the detailed interface doc into the source file.  I'll
also add some comments for describing the behavior of the internals.

Thanks,
Jason
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 1/7] Add generic, reusable menu code

2011-07-25 Thread Wolfgang Denk
Dear Jason Hobbs,

In message 1309364719-16219-2-git-send-email-jason.ho...@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.ho...@calxeda.com
 ---
 changes in v2:
  - new in v2
 
 changes in v3:
  - move timeout support to later patch
  - fix NULL case bug in menu_item_key_match
  - consistently use 'item_key' instead of 'key'
  - move default/prompt logic into menu code
  - consistently return int for error propagation
  - change option setting functions to menu_create paramaters
  - add README.menu
 
  common/Makefile |1 +
  common/menu.c   |  266 
 +++
  doc/README.menu |  158 
  include/menu.h  |   30 ++
  4 files changed, 455 insertions(+), 0 deletions(-)
  create mode 100644 common/menu.c
  create mode 100644 doc/README.menu
  create mode 100644 include/menu.h

I am happy that you provide documentation in doc/README.menu, but I
really dislike that the code itself is basicly uncommented. It is a
major pain to have to switch between the README and the source files
when trying to understand the code.

Please add sufficient comments to the code to make it readable.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
It is practically impossible to teach good programming style to  stu-
dents that have had prior exposure to BASIC: as potential programmers
they are mentally mutilated beyond hope of regeneration.   - Dijkstra
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v3 1/7] Add generic, reusable menu code

2011-06-29 Thread Jason Hobbs
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.ho...@calxeda.com
---
changes in v2:
 - new in v2

changes in v3:
 - move timeout support to later patch
 - fix NULL case bug in menu_item_key_match
 - consistently use 'item_key' instead of 'key'
 - move default/prompt logic into menu code
 - consistently return int for error propagation
 - change option setting functions to menu_create paramaters
 - add README.menu

 common/Makefile |1 +
 common/menu.c   |  266 +++
 doc/README.menu |  158 
 include/menu.h  |   30 ++
 4 files changed, 455 insertions(+), 0 deletions(-)
 create mode 100644 common/menu.c
 create mode 100644 doc/README.menu
 create mode 100644 include/menu.h

diff --git a/common/Makefile b/common/Makefile
index 224b7cc..d5bd983 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -170,6 +170,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 000..9bcd906
--- /dev/null
+++ b/common/menu.c
@@ -0,0 +1,266 @@
+/*
+ * 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;
+   char *title;
+   int prompt;
+   void (*item_data_print)(void *);
+   struct list_head items;
+};
+
+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 (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 *extra)
+{
+   char *item_key = extra;
+
+   if (!item_key || !item-key) {
+   if (item_key == item-key)
+   return item;
+
+   return NULL;
+   }
+
+   if (strcmp(item-key, item_key) == 0)
+   return item;
+
+   return NULL;
+}
+
+static inline struct menu_item *menu_item_by_key(struct menu *m,
+   char *item_key)
+{
+   return menu_items_iter(m, menu_item_key_match, item_key);
+}
+
+static inline int menu_use_default(struct menu *m)
+{
+   return !m-prompt;
+}
+
+static inline int menu_default_choice(struct menu *m, void **choice)
+{
+   if (m-default_item) {
+   *choice = m-default_item-data;
+   return 1;
+   }
+
+   return -ENOENT;
+}
+
+static inline int menu_interactive_choice(struct menu *m, void **choice)
+{
+   char cbuf[CONFIG_SYS_CBSIZE];
+   struct menu_item *choice_item = NULL;
+   int readret;
+
+   while (!choice_item) {
+