[U-Boot] [RFC PATCH 0/5] Add environment call-back capability

[U-Boot] [RFC PATCH 0/5] Add environment call-back capability

[Joe Hershberger]

When a variable with a registered callback is inserted, deleted, or
overwritten the callback is called and gives the system an opportunity
to do something in response to the change.  It also has the opportunuty
to reject the change by returning non-zero.

Before I go much further, I want to get a little feedback if this is a
good implementation.  It certainly cleans up cmd_nvedit.c significantly!


Joe Hershberger (5):
  env: Add support for callbacks to environment vars
  env: Add a loadaddr env handler
  env: Add a bootfile env handler
  env: Add a baudrate env handler
  env: Add a console env handler

 arch/arm/cpu/u-boot.lds |   7 +++
 common/Makefile         |   4 +-
 common/cmd_load.c       |  24 +++++++-
 common/cmd_nvedit.c     |  87 --------------------------
 common/console.c        |  46 ++++++++++++++
 common/env_attr.c       | 159 ++++++++++++++++++++++++++++++++++++++++++++++++
 common/env_callback.c   | 114 ++++++++++++++++++++++++++++++++++
 common/serial.c         |  58 ++++++++++++++++++
 include/env_attr.h      |  35 +++++++++++
 include/env_callback.h  |  64 +++++++++++++++++++
 include/environment.h   |   2 +
 include/search.h        |   5 ++
 lib/hashtable.c         |  65 +++++++++++++++++++-
 net/net.c               |  16 +++++
 14 files changed, 595 insertions(+), 91 deletions(-)
 create mode 100644 common/env_attr.c
 create mode 100644 common/env_callback.c
 create mode 100644 include/env_attr.h
 create mode 100644 include/env_callback.h

-- 
1.7.11.5

[U-Boot] [RFC PATCH 1/5] env: Add support for callbacks to environment vars

[Joe Hershberger]

Add support for callbacks to the "hashtable" functions.

One check-patch warning to inter-op with existing hashtable code:
WARNING: do not add new typedefs
+typedef struct entry ENTRY;

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 arch/arm/cpu/u-boot.lds |   7 +++
 common/Makefile         |   2 +
 common/env_attr.c       | 159 ++++++++++++++++++++++++++++++++++++++++++++++++
 common/env_callback.c   | 114 ++++++++++++++++++++++++++++++++++
 include/env_attr.h      |  35 +++++++++++
 include/env_callback.h  |  62 +++++++++++++++++++
 include/environment.h   |   2 +
 include/search.h        |   5 ++
 lib/hashtable.c         |  65 +++++++++++++++++++-
 9 files changed, 450 insertions(+), 1 deletion(-)
 create mode 100644 common/env_attr.c
 create mode 100644 common/env_callback.c
 create mode 100644 include/env_attr.h
 create mode 100644 include/env_callback.h

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index e49ca0c..5af8c36 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -55,6 +55,13 @@ SECTIONS
 
 	. = ALIGN(4);
 
+	. = .;
+	__u_boot_env_clbk_start = .;
+	.u_boot_env_clbk : { *(.u_boot_env_clbk) }
+	__u_boot_env_clbk_end = .;
+
+	. = ALIGN(4);
+
 	__image_copy_end = .;
 
 	.rel.dyn : {
diff --git a/common/Makefile b/common/Makefile
index 3d62775..f61c9a1 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -43,7 +43,9 @@ COBJS-y += cmd_nvedit.o
 COBJS-y += cmd_version.o
 
 # environment
+COBJS-y += env_attr.o
 COBJS-y += env_common.o
+COBJS-y += env_callback.o
 COBJS-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o
 COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o
 XCOBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
diff --git a/common/env_attr.c b/common/env_attr.c
new file mode 100644
index 0000000..b5a8bec
--- /dev/null
+++ b/common/env_attr.c
@@ -0,0 +1,159 @@
+/*
+ * (C) Copyright 2012
+ * Joe Hershberger, National Instruments, joe.hershberger at ni.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <environment.h>
+#include <errno.h>
+#include <malloc.h>
+
+/*
+ * Iterate through the whole list calling the callback for each found element.
+ */
+int env_attr_walk(const char *attr_list,
+	int (*callback)(const char *name, const char *value))
+{
+	const char *entry, *entry_end;
+	char *name, *value;
+
+	if (!attr_list)
+		/* list not found */
+		return 1;
+
+	entry = attr_list;
+	do {
+		char *entry_cpy = NULL;
+
+		entry_end = strchr(entry, ENV_ATTR_LIST_DELIM);
+		if (entry_end == NULL) {
+			int entry_len = strlen(entry);
+
+			if (entry_len) {
+				entry_cpy = malloc(entry_len + 1);
+				if (entry_cpy)
+					strcpy(entry_cpy, entry);
+				else
+					return -ENOMEM;
+			}
+		} else {
+			int entry_len = entry_end - entry;
+
+			if (entry_len) {
+				entry_cpy = malloc(entry_len + 1);
+				if (entry_cpy) {
+					strncpy(entry_cpy, entry, entry_len);
+					entry_cpy[entry_len] = '\0';
+				} else
+					return -ENOMEM;
+			}
+		}
+
+		if (entry_cpy != NULL) {
+			value = strchr(entry_cpy, ENV_ATTR_SEP);
+			if (value != NULL) {
+				*value++ = '\0';
+				value = strim(value);
+			}
+			name = strim(entry_cpy);
+
+			if (strlen(name) != 0) {
+				int retval = 0;
+
+				retval = callback(name, value);
+				if (retval) {
+					free(entry_cpy);
+					return retval;
+				}
+			}
+		}
+
+		free(entry_cpy);
+		entry = entry_end + 1;
+	} while (entry_end != NULL);
+
+	return 0;
+}
+
+/*
+ * Retrieve the attributes string associated with a single name in the list
+ * There is no protection on attributes being too small for the value
+ */
+int env_attr_lookup(const char *attr_list, const char *name, char *attributes)
+{
+	const char *entry = NULL;
+
+	if (!attributes)
+		/* bad parameter */
+		return -1;
+	if (!attr_list)
+		/* list not found */
+		return 1;
+
+	entry = strstr(attr_list, name);
+	while (entry != NULL) {
+		if ((entry == attr_list ||
+		    *(entry - 1) == ENV_ATTR_LIST_DELIM ||
+		    *(entry - 1) == ' ') &&
+		    (*(entry + strlen(name)) == ENV_ATTR_SEP ||
+		     *(entry + strlen(name)) == ENV_ATTR_LIST_DELIM ||
+		     *(entry + strlen(name)) == '\0' ||
+		     *(entry + strlen(name)) == ' '))
+			break;
+		entry++;
+		entry = strstr(entry, name);
+	}
+	if (entry != NULL) {
+		int len;
+
+		/* skip the name */
+		entry += strlen(name);
+		/* skip spaces */
+		while (*entry == ' ')
+			entry++;
+		if (*entry != ENV_ATTR_SEP)
+			len = 0;
+		else {
+			const char *delim;
+			const char delims[2] = {ENV_ATTR_LIST_DELIM, ' '};
+
+			/* skip the attr sep */
+			entry += 1;
+			/* skip spaces */
+			while (*entry == ' ')
+				entry++;
+
+			delim = strpbrk(entry, delims);
+			if (delim == NULL)
+				len = strlen(entry);
+			else
+				len = delim - entry;
+			memcpy(attributes, entry, len);
+		}
+		attributes[len] = '\0';
+
+		/* success */
+		return 0;
+	}
+
+	/* not found in list */
+	return 2;
+}
diff --git a/common/env_callback.c b/common/env_callback.c
new file mode 100644
index 0000000..9b9c915
--- /dev/null
+++ b/common/env_callback.c
@@ -0,0 +1,114 @@
+/*
+ * (C) Copyright 2012
+ * Joe Hershberger, National Instruments, joe.hershberger at ni.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <environment.h>
+
+/*
+ * Look up a callback function pointer by name
+ */
+struct env_clbk_tbl *find_env_callback(const char *name)
+{
+	struct env_clbk_tbl *clbkp;
+
+	if (name == NULL)
+		return NULL;
+
+	for (clbkp = &__u_boot_env_clbk_start;
+	     clbkp != &__u_boot_env_clbk_end;
+	     clbkp++) {
+		if (strcmp(name, clbkp->name) == 0)
+			return clbkp;
+	}
+
+	return NULL;
+}
+
+/*
+ * Look for a possible callback for a newly added variable
+ * This is called specifically when the variable did not exist in the hash
+ * previously, so the blanket update did not find this variable.
+ */
+void env_callback_bind(ENTRY *var_entry)
+{
+	const char *var_name = var_entry->key;
+	const char *callback_list = getenv(ENV_CALLBACK_VAR);
+	char callback_name[256] = "";
+	struct env_clbk_tbl *clbkp;
+	int ret = 1;
+
+	if (callback_list != NULL)
+		ret = env_attr_lookup(callback_list, var_name, callback_name);
+
+	if (ret)
+		ret = env_attr_lookup(ENV_CALLBACK_LIST_STATIC, var_name,
+			callback_name);
+
+	if (!ret && strlen(callback_name)) {
+		clbkp = find_env_callback(callback_name);
+
+		if (clbkp != NULL)
+			var_entry->callback = clbkp->callback;
+	}
+}
+
+static int clear_callbacks(ENTRY *entry)
+{
+	entry->callback = NULL;
+
+	return 0;
+}
+
+static int set_callbacks(const char *name, const char *value)
+{
+	ENTRY e, *ep;
+	struct env_clbk_tbl *clbkp;
+
+	e.key	= name;
+	e.data	= NULL;
+	hsearch_r(e, FIND, &ep, &env_htab);
+
+	if (ep != NULL) {
+		if (value == NULL || strlen(value) == 0)
+			ep->callback = NULL;
+		else {
+			clbkp = find_env_callback(value);
+
+			if (clbkp != NULL)
+				ep->callback = clbkp->callback;
+		}
+	}
+
+	return 0;
+}
+
+static int on_callbacks(const char *name, const char *value, enum env_op op)
+{
+	hwalk_r(&env_htab, clear_callbacks);
+
+	env_attr_walk(ENV_CALLBACK_LIST_STATIC, set_callbacks);
+	env_attr_walk(value, set_callbacks);
+
+	return 0;
+}
+U_BOOT_ENV_CALLBACK(callbacks, on_callbacks);
diff --git a/include/env_attr.h b/include/env_attr.h
new file mode 100644
index 0000000..62a3667
--- /dev/null
+++ b/include/env_attr.h
@@ -0,0 +1,35 @@
+/*
+ * (C) Copyright 2012
+ * Joe Hershberger, National Instruments, joe.hershberger at ni.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __ENV_ATTR_H__
+#define __ENV_ATTR_H__
+
+#define ENV_ATTR_LIST_DELIM	','
+#define ENV_ATTR_SEP		':'
+
+extern int env_attr_walk(const char *attr_list,
+	int (*callback)(const char *name, const char *value));
+extern int env_attr_lookup(const char *attr_list, const char *name,
+	char *attributes);
+
+#endif /* __ENV_ATTR_H__ */
diff --git a/include/env_callback.h b/include/env_callback.h
new file mode 100644
index 0000000..4a92c24
--- /dev/null
+++ b/include/env_callback.h
@@ -0,0 +1,62 @@
+/*
+ * (C) Copyright 2012
+ * Joe Hershberger, National Instruments, joe.hershberger at ni.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __ENV_CALLBACK_H__
+#define __ENV_CALLBACK_H__
+
+#define ENV_CALLBACK_VAR ".callbacks"
+
+#ifndef CONFIG_ENV_CALLBACK_LIST_STATIC
+#define CONFIG_ENV_CALLBACK_LIST_STATIC
+#endif
+
+#define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
+	CONFIG_ENV_CALLBACK_LIST_STATIC
+
+enum env_op {
+	env_op_none,
+	env_op_create,
+	env_op_delete,
+	env_op_overwrite,
+};
+
+struct env_clbk_tbl {
+	const char *name;		/* Callback name */
+	int (*callback)(const char *name, const char *value, enum env_op op);
+};
+
+extern struct env_clbk_tbl __u_boot_env_clbk_start;
+extern struct env_clbk_tbl __u_boot_env_clbk_end;
+
+struct env_clbk_tbl *find_env_callback(const char *);
+typedef struct entry ENTRY;
+void env_callback_bind(ENTRY *var_entry);
+
+#define ENV_CLBK_SECTION __attribute__((unused, section(".u_boot_env_clbk"), \
+		aligned(4)))
+
+#define U_BOOT_ENV_CALLBACK(name, callback) \
+	struct env_clbk_tbl __u_boot_env_clbk_##name ENV_CLBK_SECTION = \
+	{#name, callback}
+
+#endif /* __ENV_CALLBACK_H__ */
diff --git a/include/environment.h b/include/environment.h
index ae3f7b6..040352f 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -164,6 +164,8 @@ extern void env_reloc(void);
 
 #ifndef DO_DEPS_ONLY
 
+#include <env_attr.h>
+#include <env_callback.h>
 #include <search.h>
 
 extern struct hsearch_data env_htab;
diff --git a/include/search.h b/include/search.h
index ef53edb..9d0d498 100644
--- a/include/search.h
+++ b/include/search.h
@@ -29,6 +29,7 @@
 #define	_SEARCH_H 1
 
 #include <stddef.h>
+#include <env_callback.h>
 
 #define __set_errno(val) do { errno = val; } while (0)
 
@@ -41,6 +42,7 @@ typedef enum {
 typedef struct entry {
 	const char *key;
 	char *data;
+	int (*callback)(const char *name, const char *value, enum env_op op);
 } ENTRY;
 
 /* Opaque type for internal use.  */
@@ -98,6 +100,9 @@ extern int himport_r(struct hsearch_data *__htab,
 		     const char *__env, size_t __size, const char __sep,
 		     int __flag);
 
+/* Walk the whole table calling the callback on each element */
+extern int hwalk_r(struct hsearch_data *__htab, int (*callback)(ENTRY *));
+
 /* Flags for himport_r() */
 #define	H_NOCLEAR	1	/* do not clear hash table before importing */
 
diff --git a/lib/hashtable.c b/lib/hashtable.c
index abd61c8..22ff617 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -54,7 +54,8 @@
 #define	CONFIG_ENV_MAX_ENTRIES 512
 #endif
 
-#include "search.h"
+#include <env_callback.h>
+#include <search.h>
 
 /*
  * [Aho,Sethi,Ullman] Compilers: Principles, Techniques and Tools, 1986
@@ -290,6 +291,15 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
 		    && strcmp(item.key, htab->table[idx].entry.key) == 0) {
 			/* Overwrite existing value? */
 			if ((action == ENTER) && (item.data != NULL)) {
+				/* If there is a callback, call it */
+				if (htab->table[idx].entry.callback &&
+				    htab->table[idx].entry.callback(
+				    item.key, item.data, env_op_overwrite)) {
+					__set_errno(EINVAL);
+					*retval = NULL;
+					return 0;
+				}
+
 				free(htab->table[idx].entry.data);
 				htab->table[idx].entry.data =
 					strdup(item.data);
@@ -332,6 +342,16 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
 			    && strcmp(item.key, htab->table[idx].entry.key) == 0) {
 				/* Overwrite existing value? */
 				if ((action == ENTER) && (item.data != NULL)) {
+					/* If there is a callback, call it */
+					if (htab->table[idx].entry.callback &&
+					    htab->table[idx].entry.callback(
+					    item.key, item.data,
+					    env_op_overwrite)) {
+						__set_errno(EINVAL);
+						*retval = NULL;
+						return 0;
+					}
+
 					free(htab->table[idx].entry.data);
 					htab->table[idx].entry.data =
 						strdup(item.data);
@@ -380,6 +400,18 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
 
 		++htab->filled;
 
+		/* This is a new entry, so look up a possible callback */
+		env_callback_bind(&htab->table[idx].entry);
+
+		/* If there is a callback, call it */
+		if (htab->table[idx].entry.callback &&
+		    htab->table[idx].entry.callback(item.key, item.data,
+		    env_op_create)) {
+			__set_errno(EINVAL);
+			*retval = NULL;
+			return 0;
+		}
+
 		/* return new entry */
 		*retval = &htab->table[idx].entry;
 		return 1;
@@ -415,6 +447,13 @@ int hdelete_r(const char *key, struct hsearch_data *htab)
 		return 0;	/* not found */
 	}
 
+	/* If there is a callback, call it */
+	if (htab->table[idx].entry.callback &&
+	    htab->table[idx].entry.callback(key, NULL, env_op_delete)) {
+		__set_errno(EINVAL);
+		return 0;
+	}
+
 	/* free used ENTRY */
 	debug("hdelete: DELETING key \"%s\"\n", key);
 
@@ -765,3 +804,27 @@ int himport_r(struct hsearch_data *htab,
 	debug("INSERT: done\n");
 	return 1;		/* everything OK */
 }
+
+/*
+ * hwalk_r()
+ */
+
+/*
+ * Walk all of the entries in the hash, calling the callback for each one.
+ * this allows some generic operation to be performed on each element.
+ */
+int hwalk_r(struct hsearch_data *htab, int (*callback)(ENTRY *))
+{
+	int i;
+	int retval;
+
+	for (i = 1; i <= htab->size; ++i) {
+		if (htab->table[i].used > 0) {
+			retval = callback(&htab->table[i].entry);
+			if (retval)
+				return retval;
+		}
+	}
+
+	return 0;
+}
-- 
1.7.11.5

[U-Boot] [RFC PATCH 1/5] env: Add support for callbacks to environment vars

[Wolfgang Denk]

Dear Joe Hershberger,

In message <1348264998-27323-2-git-send-email-joe.hershberger@ni.com> you wrote:
> Add support for callbacks to the "hashtable" functions.
...
> +/*
> + * Iterate through the whole list calling the callback for each found element.
> + */
> +int env_attr_walk(const char *attr_list,
> +	int (*callback)(const char *name, const char *value))
> +{
> +	const char *entry, *entry_end;
> +	char *name, *value;
> +
> +	if (!attr_list)
> +		/* list not found */
> +		return 1;
> +
> +	entry = attr_list;
> +	do {
> +		char *entry_cpy = NULL;
> +
> +		entry_end = strchr(entry, ENV_ATTR_LIST_DELIM);
> +		if (entry_end == NULL) {
> +			int entry_len = strlen(entry);
> +
> +			if (entry_len) {
> +				entry_cpy = malloc(entry_len + 1);
--------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +				if (entry_cpy)
> +					strcpy(entry_cpy, entry);
> +				else
> +					return -ENOMEM;
> +			}
> +		} else {
> +			int entry_len = entry_end - entry;
> +
> +			if (entry_len) {
> +				entry_cpy = malloc(entry_len + 1);
--------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +				if (entry_cpy) {
> +					strncpy(entry_cpy, entry, entry_len);
> +					entry_cpy[entry_len] = '\0';
> +				} else
> +					return -ENOMEM;
> +			}
> +		}

I find it a bit strange that a function which walks an existing list
of data structures would malloc() anything?

> +		if (entry_cpy != NULL) {
> +			value = strchr(entry_cpy, ENV_ATTR_SEP);
> +			if (value != NULL) {
> +				*value++ = '\0';
> +				value = strim(value);
> +			}
> +			name = strim(entry_cpy);
> +
> +			if (strlen(name) != 0) {
> +				int retval = 0;
> +
> +				retval = callback(name, value);
> +				if (retval) {
> +					free(entry_cpy);
> +					return retval;
> +				}
> +			}
> +		}
> +
> +		free(entry_cpy);
> +		entry = entry_end + 1;
> +	} while (entry_end != NULL);
> +
> +	return 0;
> +}

Actually, this function is unreadable to me, especially because there
is zero documentation about what exactly it is supposed to do,  which
data formats are being used, etc.

> +/*
> + * Retrieve the attributes string associated with a single name in the list
> + * There is no protection on attributes being too small for the value
> + */
> +int env_attr_lookup(const char *attr_list, const char *name, char *attributes)

Atttributes in a list?

What exactly has this to do with implementing callbacks?

[Yes, I think I do know what you have in mind, but this is based on a
lot of speculation, and I may be wrong.  And people who didn't follow
the previous discussion and the old patch seris probably have no clue
at all.]

> +	entry = strstr(attr_list, name);
> +	while (entry != NULL) {
> +		if ((entry == attr_list ||
> +		    *(entry - 1) == ENV_ATTR_LIST_DELIM ||
> +		    *(entry - 1) == ' ') &&
> +		    (*(entry + strlen(name)) == ENV_ATTR_SEP ||
> +		     *(entry + strlen(name)) == ENV_ATTR_LIST_DELIM ||
> +		     *(entry + strlen(name)) == '\0' ||
> +		     *(entry + strlen(name)) == ' '))

Factor out strlen() ?

...
> diff --git a/include/env_attr.h b/include/env_attr.h
> new file mode 100644
> index 0000000..62a3667
> --- /dev/null
> +++ b/include/env_attr.h
...
> +#define ENV_ATTR_LIST_DELIM	','
> +#define ENV_ATTR_SEP		':'
> +
> +extern int env_attr_walk(const char *attr_list,
> +	int (*callback)(const char *name, const char *value));
> +extern int env_attr_lookup(const char *attr_list, const char *name,
> +	char *attributes);

This does need documentation...

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: wd at denx.de
You're dead, Jim.
	-- McCoy, "The Tholian Web", stardate unknown

[U-Boot] [RFC PATCH v2 0/5] Add environment call-back capability

[Joe Hershberger]

When a variable with a registered callback is inserted, deleted, or
overwritten the callback is called and gives the system an opportunity
to do something in response to the change.  It also has the opportunuty
to reject the change by returning non-zero.

Before I go much further, I want to get a little feedback if this is a
good implementation.  It certainly cleans up cmd_nvedit.c significantly!

Changes in v2:
- Added much-needed documentation
- Factored out prevch and nextch in env_attr_lookup()

Joe Hershberger (5):
  env: Add support for callbacks to environment vars
  env: Add a loadaddr env handler
  env: Add a bootfile env handler
  env: Add a baudrate env handler
  env: Add a console env handler

 README                  |  28 ++++++++
 arch/arm/cpu/u-boot.lds |   7 ++
 common/Makefile         |   4 +-
 common/cmd_load.c       |  24 ++++++-
 common/cmd_nvedit.c     |  87 ----------------------
 common/console.c        |  46 ++++++++++++
 common/env_attr.c       | 187 ++++++++++++++++++++++++++++++++++++++++++++++++
 common/env_callback.c   | 130 +++++++++++++++++++++++++++++++++
 common/serial.c         |  58 +++++++++++++++
 include/env_attr.h      |  55 ++++++++++++++
 include/env_callback.h  |  74 +++++++++++++++++++
 include/environment.h   |   2 +
 include/search.h        |   5 ++
 lib/hashtable.c         |  65 ++++++++++++++++-
 net/net.c               |  16 +++++
 15 files changed, 697 insertions(+), 91 deletions(-)
 create mode 100644 common/env_attr.c
 create mode 100644 common/env_callback.c
 create mode 100644 include/env_attr.h
 create mode 100644 include/env_callback.h

-- 
1.7.11.5

[U-Boot] [RFC PATCH v2 1/5] env: Add support for callbacks to environment vars

[Joe Hershberger]

Add support for callbacks to the "hashtable" functions.

One check-patch warning to inter-op with existing hashtable code:
WARNING: do not add new typedefs
+typedef struct entry ENTRY;

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
Changes in v2:
- Added much-needed documentation
- Factored out prevch and nextch in env_attr_lookup()

 README                  |  28 ++++++++
 arch/arm/cpu/u-boot.lds |   7 ++
 common/Makefile         |   2 +
 common/env_attr.c       | 187 ++++++++++++++++++++++++++++++++++++++++++++++++
 common/env_callback.c   | 130 +++++++++++++++++++++++++++++++++
 include/env_attr.h      |  55 ++++++++++++++
 include/env_callback.h  |  72 +++++++++++++++++++
 include/environment.h   |   2 +
 include/search.h        |   5 ++
 lib/hashtable.c         |  65 ++++++++++++++++-
 10 files changed, 552 insertions(+), 1 deletion(-)
 create mode 100644 common/env_attr.c
 create mode 100644 common/env_callback.c
 create mode 100644 include/env_attr.h
 create mode 100644 include/env_callback.h

diff --git a/README b/README
index fb9d904..47bed09 100644
--- a/README
+++ b/README
@@ -3991,6 +3991,34 @@ Please note that changes to some configuration parameters may take
 only effect after the next boot (yes, that's just like Windoze :-).
 
 
+Callback functions for environment variables:
+---------------------------------------------
+
+For some environment variables, the behavior of u-boot needs to change
+when their values are changed.  This functionailty allows functions to
+be associated with arbitrary variables.  On creation, overwrite, or
+deletion, the callback will provide the opportunity for some side
+effect to happen or for the change to be rejected.
+
+The callbacks are named and associated with a function using the
+U_BOOT_ENV_CALLBACK macro in your board or driver code.
+
+These callbacks are associated with variables in one of two ways.  The
+static list can be added to by defining CONFIG_ENV_CALLBACK_LIST_STATIC
+in the board configuration to a string that defines a list of
+associations.  The list must be in the following format:
+
+	entry = variable_name[:callback_name]
+	list = entry[,list]
+
+If the callback name is not specified, then the callback is deleted.
+Spaces are also allowed anywhere in the list.
+
+Callbacks can also be associated by defining the ".callbacks" variable
+with the same list format above.  Any association in ".callbacks" will
+override any association in the static list.
+
+
 Command Line Parsing:
 =====================
 
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index e49ca0c..5af8c36 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -55,6 +55,13 @@ SECTIONS
 
 	. = ALIGN(4);
 
+	. = .;
+	__u_boot_env_clbk_start = .;
+	.u_boot_env_clbk : { *(.u_boot_env_clbk) }
+	__u_boot_env_clbk_end = .;
+
+	. = ALIGN(4);
+
 	__image_copy_end = .;
 
 	.rel.dyn : {
diff --git a/common/Makefile b/common/Makefile
index 3d62775..f61c9a1 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -43,7 +43,9 @@ COBJS-y += cmd_nvedit.o
 COBJS-y += cmd_version.o
 
 # environment
+COBJS-y += env_attr.o
 COBJS-y += env_common.o
+COBJS-y += env_callback.o
 COBJS-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o
 COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o
 XCOBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
diff --git a/common/env_attr.c b/common/env_attr.c
new file mode 100644
index 0000000..62643c8
--- /dev/null
+++ b/common/env_attr.c
@@ -0,0 +1,187 @@
+/*
+ * (C) Copyright 2012
+ * Joe Hershberger, National Instruments, joe.hershberger at ni.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <environment.h>
+#include <errno.h>
+#include <malloc.h>
+
+/*
+ * Iterate through the whole list calling the callback for each found element.
+ * "attr_list" takes the form:
+ *	attributes = [^,:\s]*
+ *	entry = name[:attributes]
+ *	list = entry[,list]
+ */
+int env_attr_walk(const char *attr_list,
+	int (*callback)(const char *name, const char *attributes))
+{
+	const char *entry, *entry_end;
+	char *name, *attributes;
+
+	if (!attr_list)
+		/* list not found */
+		return 1;
+
+	entry = attr_list;
+	do {
+		char *entry_cpy = NULL;
+
+		entry_end = strchr(entry, ENV_ATTR_LIST_DELIM);
+		/* check if this is the last entry in the list */
+		if (entry_end == NULL) {
+			int entry_len = strlen(entry);
+
+			if (entry_len) {
+				/*
+				 * allocate memory to copy the entry into since
+				 * we will need to inject '\0' chars and squash
+				 * white-space before calling the callback
+				 */
+				entry_cpy = malloc(entry_len + 1);
+				if (entry_cpy)
+					/* copy the rest of the list */
+					strcpy(entry_cpy, entry);
+				else
+					return -ENOMEM;
+			}
+		} else {
+			int entry_len = entry_end - entry;
+
+			if (entry_len) {
+				/*
+				 * allocate memory to copy the entry into since
+				 * we will need to inject '\0' chars and squash
+				 * white-space before calling the callback
+				 */
+				entry_cpy = malloc(entry_len + 1);
+				if (entry_cpy) {
+					/* copy just this entry and null term */
+					strncpy(entry_cpy, entry, entry_len);
+					entry_cpy[entry_len] = '\0';
+				} else
+					return -ENOMEM;
+			}
+		}
+
+		/* check if there is anything to process (e.g. not ",,,") */
+		if (entry_cpy != NULL) {
+			attributes = strchr(entry_cpy, ENV_ATTR_SEP);
+			/* check if there is a ':' */
+			if (attributes != NULL) {
+				/* replace the ':' with '\0' to term name */
+				*attributes++ = '\0';
+				/* remove white-space from attributes */
+				attributes = strim(attributes);
+			}
+			/* remove white-space from name */
+			name = strim(entry_cpy);
+
+			/* only call the callback if there is a name */
+			if (strlen(name) != 0) {
+				int retval = 0;
+
+				retval = callback(name, attributes);
+				if (retval) {
+					free(entry_cpy);
+					return retval;
+				}
+			}
+		}
+
+		free(entry_cpy);
+		entry = entry_end + 1;
+	} while (entry_end != NULL);
+
+	return 0;
+}
+
+/*
+ * Retrieve the attributes string associated with a single name in the list
+ * There is no protection on attributes being too small for the value
+ */
+int env_attr_lookup(const char *attr_list, const char *name, char *attributes)
+{
+	const char *entry = NULL;
+
+	if (!attributes)
+		/* bad parameter */
+		return -1;
+	if (!attr_list)
+		/* list not found */
+		return 1;
+
+	entry = strstr(attr_list, name);
+	/* TODO: Search in reverse (last entry should have priority) */
+	while (entry != NULL) {
+		char prevch = *(entry - 1);
+		char nextch = *(entry + strlen(name));
+
+		/* check for an exact match */
+		if ((entry == attr_list ||
+		     prevch == ENV_ATTR_LIST_DELIM ||
+		     prevch == ' ') &&
+		    (nextch == ENV_ATTR_SEP ||
+		     nextch == ENV_ATTR_LIST_DELIM ||
+		     nextch == '\0' ||
+		     nextch == ' '))
+			break;
+		entry++;
+		entry = strstr(entry, name);
+	}
+	if (entry != NULL) {
+		int len;
+
+		/* skip the name */
+		entry += strlen(name);
+		/* skip spaces */
+		while (*entry == ' ')
+			entry++;
+		if (*entry != ENV_ATTR_SEP)
+			len = 0;
+		else {
+			const char *delim;
+			const char delims[2] = {ENV_ATTR_LIST_DELIM, ' '};
+
+			/* skip the attr sep */
+			entry += 1;
+			/* skip spaces */
+			while (*entry == ' ')
+				entry++;
+
+			delim = strpbrk(entry, delims);
+			if (delim == NULL)
+				len = strlen(entry);
+			else
+				len = delim - entry;
+			memcpy(attributes, entry, len);
+		}
+		attributes[len] = '\0';
+
+		/* success */
+		return 0;
+	}
+
+	/* not found in list */
+	return 2;
+}
diff --git a/common/env_callback.c b/common/env_callback.c
new file mode 100644
index 0000000..8950647
--- /dev/null
+++ b/common/env_callback.c
@@ -0,0 +1,130 @@
+/*
+ * (C) Copyright 2012
+ * Joe Hershberger, National Instruments, joe.hershberger at ni.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <environment.h>
+
+/*
+ * Look up a callback function pointer by name
+ */
+struct env_clbk_tbl *find_env_callback(const char *name)
+{
+	struct env_clbk_tbl *clbkp;
+
+	if (name == NULL)
+		return NULL;
+
+	/* look up the callback in the linker-list */
+	/* TODO: replace with generic linker-list from marex */
+	for (clbkp = &__u_boot_env_clbk_start;
+	     clbkp != &__u_boot_env_clbk_end;
+	     clbkp++) {
+		if (strcmp(name, clbkp->name) == 0)
+			return clbkp;
+	}
+
+	return NULL;
+}
+
+/*
+ * Look for a possible callback for a newly added variable
+ * This is called specifically when the variable did not exist in the hash
+ * previously, so the blanket update did not find this variable.
+ */
+void env_callback_bind(ENTRY *var_entry)
+{
+	const char *var_name = var_entry->key;
+	const char *callback_list = getenv(ENV_CALLBACK_VAR);
+	char callback_name[256] = "";
+	struct env_clbk_tbl *clbkp;
+	int ret = 1;
+
+	/* look in the ".callbacks" var for a reference to this variable */
+	if (callback_list != NULL)
+		ret = env_attr_lookup(callback_list, var_name, callback_name);
+
+	/* only if not found there, look in the static list */
+	if (ret)
+		ret = env_attr_lookup(ENV_CALLBACK_LIST_STATIC, var_name,
+			callback_name);
+
+	/* if an association was found, set the callback pointer */
+	if (!ret && strlen(callback_name)) {
+		clbkp = find_env_callback(callback_name);
+		if (clbkp != NULL)
+			var_entry->callback = clbkp->callback;
+	}
+}
+
+/*
+ * Called on each existing env var prior to the blanket update since removing
+ * a callback association should remove its callback.
+ */
+static int clear_callback(ENTRY *entry)
+{
+	entry->callback = NULL;
+
+	return 0;
+}
+
+/*
+ * Call for each element in the list that associates variables to callbacks
+ */
+static int set_callback(const char *name, const char *value)
+{
+	ENTRY e, *ep;
+	struct env_clbk_tbl *clbkp;
+
+	e.key	= name;
+	e.data	= NULL;
+	hsearch_r(e, FIND, &ep, &env_htab);
+
+	/* does the env variable actually exist? */
+	if (ep != NULL) {
+		/* the assocaition delares no callback, so remove the pointer */
+		if (value == NULL || strlen(value) == 0)
+			ep->callback = NULL;
+		else {
+			/* assign the requested callback */
+			clbkp = find_env_callback(value);
+			if (clbkp != NULL)
+				ep->callback = clbkp->callback;
+		}
+	}
+
+	return 0;
+}
+
+static int on_callbacks(const char *name, const char *value, enum env_op op)
+{
+	/* remove all callbacks */
+	hwalk_r(&env_htab, clear_callback);
+
+	/* configure any static callback bindings */
+	env_attr_walk(ENV_CALLBACK_LIST_STATIC, set_callback);
+	/* configure any dynamic callback bindings */
+	env_attr_walk(value, set_callback);
+
+	return 0;
+}
+U_BOOT_ENV_CALLBACK(callbacks, on_callbacks);
diff --git a/include/env_attr.h b/include/env_attr.h
new file mode 100644
index 0000000..6ef114f
--- /dev/null
+++ b/include/env_attr.h
@@ -0,0 +1,55 @@
+/*
+ * (C) Copyright 2012
+ * Joe Hershberger, National Instruments, joe.hershberger at ni.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __ENV_ATTR_H__
+#define __ENV_ATTR_H__
+
+#define ENV_ATTR_LIST_DELIM	','
+#define ENV_ATTR_SEP		':'
+
+/*
+ * env_attr_walk takes as input an "attr_list" that takes the form:
+ *	attributes = [^,:\s]*
+ *	entry = name[:attributes]
+ *	list = entry[,list]
+ * It will call the "callback" function with the "name" and attribute as "value"
+ * The callback may return a non-0 to abort the list walk.
+ * This return value will be passed through to the caller.
+ * 0 is returned on success.
+ */
+extern int env_attr_walk(const char *attr_list,
+	int (*callback)(const char *name, const char *value));
+
+/*
+ * env_attr_lookup takes as input an "attr_list" with the same form as above.
+ * It also takes as input a "name" to look for.
+ * If the name is found in the list, it's value is copied into "attributes".
+ * There is no protection on attributes being too small for the value.
+ * It returns -1 if attributes is NULL, 1 if "name" is not found, 2 if
+ * "attr_list" is NULL.
+ * Returns 0 on success.
+ */
+extern int env_attr_lookup(const char *attr_list, const char *name,
+	char *attributes);
+
+#endif /* __ENV_ATTR_H__ */
diff --git a/include/env_callback.h b/include/env_callback.h
new file mode 100644
index 0000000..76ac865
--- /dev/null
+++ b/include/env_callback.h
@@ -0,0 +1,72 @@
+/*
+ * (C) Copyright 2012
+ * Joe Hershberger, National Instruments, joe.hershberger at ni.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __ENV_CALLBACK_H__
+#define __ENV_CALLBACK_H__
+
+#define ENV_CALLBACK_VAR ".callbacks"
+
+/* Board configs can define additional static callback bindings */
+#ifndef CONFIG_ENV_CALLBACK_LIST_STATIC
+#define CONFIG_ENV_CALLBACK_LIST_STATIC
+#endif
+
+/*
+ * This list of callback bindings is static, but may be overridden by defining
+ * a new assogiation in the ".callbacks" environment variable.
+ */
+#define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
+	CONFIG_ENV_CALLBACK_LIST_STATIC
+
+enum env_op {
+	env_op_none,
+	env_op_create,
+	env_op_delete,
+	env_op_overwrite,
+};
+
+struct env_clbk_tbl {
+	const char *name;		/* Callback name */
+	int (*callback)(const char *name, const char *value, enum env_op op);
+};
+
+extern struct env_clbk_tbl __u_boot_env_clbk_start;
+extern struct env_clbk_tbl __u_boot_env_clbk_end;
+
+struct env_clbk_tbl *find_env_callback(const char *);
+typedef struct entry ENTRY;
+void env_callback_bind(ENTRY *var_entry);
+
+#define ENV_CLBK_SECTION __attribute__((unused, section(".u_boot_env_clbk"), \
+		aligned(4)))
+
+/*
+ * Define a callback that can be associated with variables.
+ * when associated through the ".callbacks" environment variable, the callback
+ * will be executed any time the variable is inserted, overwritten, or deleted.
+ */
+#define U_BOOT_ENV_CALLBACK(name, callback) \
+	struct env_clbk_tbl __u_boot_env_clbk_##name ENV_CLBK_SECTION = \
+	{#name, callback}
+
+#endif /* __ENV_CALLBACK_H__ */
diff --git a/include/environment.h b/include/environment.h
index ae3f7b6..040352f 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -164,6 +164,8 @@ extern void env_reloc(void);
 
 #ifndef DO_DEPS_ONLY
 
+#include <env_attr.h>
+#include <env_callback.h>
 #include <search.h>
 
 extern struct hsearch_data env_htab;
diff --git a/include/search.h b/include/search.h
index ef53edb..9d0d498 100644
--- a/include/search.h
+++ b/include/search.h
@@ -29,6 +29,7 @@
 #define	_SEARCH_H 1
 
 #include <stddef.h>
+#include <env_callback.h>
 
 #define __set_errno(val) do { errno = val; } while (0)
 
@@ -41,6 +42,7 @@ typedef enum {
 typedef struct entry {
 	const char *key;
 	char *data;
+	int (*callback)(const char *name, const char *value, enum env_op op);
 } ENTRY;
 
 /* Opaque type for internal use.  */
@@ -98,6 +100,9 @@ extern int himport_r(struct hsearch_data *__htab,
 		     const char *__env, size_t __size, const char __sep,
 		     int __flag);
 
+/* Walk the whole table calling the callback on each element */
+extern int hwalk_r(struct hsearch_data *__htab, int (*callback)(ENTRY *));
+
 /* Flags for himport_r() */
 #define	H_NOCLEAR	1	/* do not clear hash table before importing */
 
diff --git a/lib/hashtable.c b/lib/hashtable.c
index abd61c8..22ff617 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -54,7 +54,8 @@
 #define	CONFIG_ENV_MAX_ENTRIES 512
 #endif
 
-#include "search.h"
+#include <env_callback.h>
+#include <search.h>
 
 /*
  * [Aho,Sethi,Ullman] Compilers: Principles, Techniques and Tools, 1986
@@ -290,6 +291,15 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
 		    && strcmp(item.key, htab->table[idx].entry.key) == 0) {
 			/* Overwrite existing value? */
 			if ((action == ENTER) && (item.data != NULL)) {
+				/* If there is a callback, call it */
+				if (htab->table[idx].entry.callback &&
+				    htab->table[idx].entry.callback(
+				    item.key, item.data, env_op_overwrite)) {
+					__set_errno(EINVAL);
+					*retval = NULL;
+					return 0;
+				}
+
 				free(htab->table[idx].entry.data);
 				htab->table[idx].entry.data =
 					strdup(item.data);
@@ -332,6 +342,16 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
 			    && strcmp(item.key, htab->table[idx].entry.key) == 0) {
 				/* Overwrite existing value? */
 				if ((action == ENTER) && (item.data != NULL)) {
+					/* If there is a callback, call it */
+					if (htab->table[idx].entry.callback &&
+					    htab->table[idx].entry.callback(
+					    item.key, item.data,
+					    env_op_overwrite)) {
+						__set_errno(EINVAL);
+						*retval = NULL;
+						return 0;
+					}
+
 					free(htab->table[idx].entry.data);
 					htab->table[idx].entry.data =
 						strdup(item.data);
@@ -380,6 +400,18 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
 
 		++htab->filled;
 
+		/* This is a new entry, so look up a possible callback */
+		env_callback_bind(&htab->table[idx].entry);
+
+		/* If there is a callback, call it */
+		if (htab->table[idx].entry.callback &&
+		    htab->table[idx].entry.callback(item.key, item.data,
+		    env_op_create)) {
+			__set_errno(EINVAL);
+			*retval = NULL;
+			return 0;
+		}
+
 		/* return new entry */
 		*retval = &htab->table[idx].entry;
 		return 1;
@@ -415,6 +447,13 @@ int hdelete_r(const char *key, struct hsearch_data *htab)
 		return 0;	/* not found */
 	}
 
+	/* If there is a callback, call it */
+	if (htab->table[idx].entry.callback &&
+	    htab->table[idx].entry.callback(key, NULL, env_op_delete)) {
+		__set_errno(EINVAL);
+		return 0;
+	}
+
 	/* free used ENTRY */
 	debug("hdelete: DELETING key \"%s\"\n", key);
 
@@ -765,3 +804,27 @@ int himport_r(struct hsearch_data *htab,
 	debug("INSERT: done\n");
 	return 1;		/* everything OK */
 }
+
+/*
+ * hwalk_r()
+ */
+
+/*
+ * Walk all of the entries in the hash, calling the callback for each one.
+ * this allows some generic operation to be performed on each element.
+ */
+int hwalk_r(struct hsearch_data *htab, int (*callback)(ENTRY *))
+{
+	int i;
+	int retval;
+
+	for (i = 1; i <= htab->size; ++i) {
+		if (htab->table[i].used > 0) {
+			retval = callback(&htab->table[i].entry);
+			if (retval)
+				return retval;
+		}
+	}
+
+	return 0;
+}
-- 
1.7.11.5

[U-Boot] [RFC PATCH v2 1/5] env: Add support for callbacks to environment vars

[Marek Vasut]

Dear Joe Hershberger,

[...]

> +	. = .;
> +	__u_boot_env_clbk_start = .;
> +	.u_boot_env_clbk : { *(.u_boot_env_clbk) }
> +	__u_boot_env_clbk_end = .;
> +
> +	. = ALIGN(4);
> +
[...]

Can you make some use of the LG-array patches ?

Best regards,
Marek Vasut

[U-Boot] [RFC PATCH v2 2/5] env: Add a loadaddr env handler

[Joe Hershberger]

Remove the hard-coded loadaddr handler and use a callback instead

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 common/cmd_load.c      | 24 ++++++++++++++++++++++--
 common/cmd_nvedit.c    | 10 +---------
 include/env_callback.h |  1 +
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/common/cmd_load.c b/common/cmd_load.c
index f4d66de..8849596 100644
--- a/common/cmd_load.c
+++ b/common/cmd_load.c
@@ -30,9 +30,14 @@
 #include <net.h>
 #include <exports.h>
 #include <xyzModem.h>
+#include <environment.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+ulong load_addr = CONFIG_SYS_LOAD_ADDR;	/* Default Load Address */
+ulong save_addr;			/* Default Save Address */
+ulong save_size;			/* Default Save Size (in bytes) */
+
 #if defined(CONFIG_CMD_LOADB)
 static ulong load_serial_ymodem (ulong offset);
 #endif
@@ -50,6 +55,21 @@ static int do_echo = 1;
 
 /* -------------------------------------------------------------------- */
 
+static int on_loadaddr(const char *name, const char *value, enum env_op op)
+{
+	switch (op) {
+	case env_op_create:
+	case env_op_overwrite:
+		load_addr = simple_strtoul(value, NULL, 16);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+U_BOOT_ENV_CALLBACK(loadaddr, on_loadaddr);
+
 #if defined(CONFIG_CMD_LOADS)
 int do_load_serial (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
@@ -257,8 +277,8 @@ read_record (char *buf, ulong len)
 
 int do_save_serial (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	ulong offset = 0;
-	ulong size   = 0;
+	ulong offset = save_addr;
+	ulong size   = save_size;
 #ifdef	CONFIG_SYS_LOADS_BAUD_CHANGE
 	int save_baudrate, current_baudrate;
 
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index fd05e72..9e8ea12 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -79,10 +79,6 @@ SPI_FLASH|NVRAM|MMC|FAT|REMOTE} or CONFIG_ENV_IS_NOWHERE
  */
 #define	MAX_ENV_SIZE	(1 << 20)	/* 1 MiB */
 
-ulong load_addr = CONFIG_SYS_LOAD_ADDR;	/* Default Load Address */
-ulong save_addr;			/* Default Save Address */
-ulong save_size;			/* Default Save Size (in bytes) */
-
 /*
  * Table with supported baudrates (defined in config_xyz.h)
  */
@@ -341,12 +337,8 @@ int _do_env_set(int flag, int argc, char * const argv[])
 	 * Some variables should be updated when the corresponding
 	 * entry in the environment is changed
 	 */
-	if (strcmp(argv[1], "loadaddr") == 0) {
-		load_addr = simple_strtoul(argv[2], NULL, 16);
-		return 0;
-	}
 #if defined(CONFIG_CMD_NET)
-	else if (strcmp(argv[1], "bootfile") == 0) {
+	if (strcmp(argv[1], "bootfile") == 0) {
 		copy_filename(BootFile, argv[2], sizeof(BootFile));
 		return 0;
 	}
diff --git a/include/env_callback.h b/include/env_callback.h
index 76ac865..9c1cbe9 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -36,6 +36,7 @@
  * a new assogiation in the ".callbacks" environment variable.
  */
 #define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
+	"loadaddr:loadaddr," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 enum env_op {
-- 
1.7.11.5

[U-Boot] [RFC PATCH v2 3/5] env: Add a bootfile env handler

[Joe Hershberger]

Remove the hard-coded bootfile handler and use a callback instead

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 common/cmd_nvedit.c    | 13 -------------
 include/env_callback.h |  2 +-
 net/net.c              | 16 ++++++++++++++++
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9e8ea12..ac15a3c 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -50,9 +50,6 @@
 #include <serial.h>
 #include <linux/stddef.h>
 #include <asm/byteorder.h>
-#if defined(CONFIG_CMD_NET)
-#include <net.h>
-#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -333,16 +330,6 @@ int _do_env_set(int flag, int argc, char * const argv[])
 		return 1;
 	}
 
-	/*
-	 * Some variables should be updated when the corresponding
-	 * entry in the environment is changed
-	 */
-#if defined(CONFIG_CMD_NET)
-	if (strcmp(argv[1], "bootfile") == 0) {
-		copy_filename(BootFile, argv[2], sizeof(BootFile));
-		return 0;
-	}
-#endif
 	return 0;
 }
 
diff --git a/include/env_callback.h b/include/env_callback.h
index 9c1cbe9..b450077 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -36,7 +36,7 @@
  * a new assogiation in the ".callbacks" environment variable.
  */
 #define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
-	"loadaddr:loadaddr," \
+	"loadaddr:loadaddr,bootfile:bootfile," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 enum env_op {
diff --git a/net/net.c b/net/net.c
index e8ff066..7d7196f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -82,6 +82,7 @@
 
 #include <common.h>
 #include <command.h>
+#include <environment.h>
 #include <net.h>
 #if defined(CONFIG_STATUS_LED)
 #include <miiphy.h>
@@ -208,6 +209,21 @@ static int NetTryCount;
 
 /**********************************************************************/
 
+static int on_bootfile(const char *name, const char *value, enum env_op op)
+{
+	switch (op) {
+	case env_op_create:
+	case env_op_overwrite:
+		copy_filename(BootFile, value, sizeof(BootFile));
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+U_BOOT_ENV_CALLBACK(bootfile, on_bootfile);
+
 /*
  * Check if autoload is enabled. If so, use either NFS or TFTP to download
  * the boot file.
-- 
1.7.11.5

[U-Boot] [RFC PATCH v2 4/5] env: Add a baudrate env handler

[Joe Hershberger]

Remove the hard-coded baudrate handler and use a callback instead

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 common/Makefile        |  2 +-
 common/cmd_nvedit.c    | 35 ------------------------------
 common/serial.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/env_callback.h |  2 +-
 4 files changed, 60 insertions(+), 37 deletions(-)

diff --git a/common/Makefile b/common/Makefile
index f61c9a1..1187960 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -32,7 +32,7 @@ COBJS-y += command.o
 COBJS-y += exports.o
 COBJS-$(CONFIG_SYS_HUSH_PARSER) += hush.o
 COBJS-y += s_record.o
-COBJS-$(CONFIG_SERIAL_MULTI) += serial.o
+COBJS-y += serial.o
 COBJS-y += xyzModem.o
 
 # core command
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index ac15a3c..652e8ce 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -47,7 +47,6 @@
 #include <errno.h>
 #include <malloc.h>
 #include <watchdog.h>
-#include <serial.h>
 #include <linux/stddef.h>
 #include <asm/byteorder.h>
 
@@ -77,12 +76,6 @@ SPI_FLASH|NVRAM|MMC|FAT|REMOTE} or CONFIG_ENV_IS_NOWHERE
 #define	MAX_ENV_SIZE	(1 << 20)	/* 1 MiB */
 
 /*
- * Table with supported baudrates (defined in config_xyz.h)
- */
-static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
-#define	N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
-
-/*
  * This variable is incremented on each do_env_set(), so it can
  * be used via get_env_id() as an indication, if the environment
  * has changed or not. So it is possible to reread an environment
@@ -263,34 +256,6 @@ int _do_env_set(int flag, int argc, char * const argv[])
 			return 1;
 		}
 #endif
-		/*
-		 * Switch to new baudrate if new baudrate is supported
-		 */
-		if (strcmp(name, "baudrate") == 0) {
-			int baudrate = simple_strtoul(argv[2], NULL, 10);
-			int i;
-			for (i = 0; i < N_BAUDRATES; ++i) {
-				if (baudrate == baudrate_table[i])
-					break;
-			}
-			if (i == N_BAUDRATES) {
-				printf("## Baudrate %d bps not supported\n",
-					baudrate);
-				return 1;
-			}
-			printf("## Switch baudrate to %d bps and"
-			       "press ENTER ...\n", baudrate);
-			udelay(50000);
-			gd->baudrate = baudrate;
-#if defined(CONFIG_PPC) || defined(CONFIG_MCF52x2)
-			gd->bd->bi_baudrate = baudrate;
-#endif
-
-			serial_setbrg();
-			udelay(50000);
-			while (getc() != '\r')
-				;
-		}
 	}
 
 	/* Delete only ? */
diff --git a/common/serial.c b/common/serial.c
index 75cc1bb..4ff1463 100644
--- a/common/serial.c
+++ b/common/serial.c
@@ -22,6 +22,7 @@
  */
 
 #include <common.h>
+#include <environment.h>
 #include <serial.h>
 #include <stdio_dev.h>
 #include <post.h>
@@ -29,6 +30,62 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/*
+ * Table with supported baudrates (defined in config_xyz.h)
+ */
+static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
+#define	N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
+
+static int on_baudrate(const char *name, const char *value, enum env_op op)
+{
+	int i;
+	int baudrate;
+
+	switch (op) {
+	case env_op_create:
+	case env_op_overwrite:
+		/*
+		 * Switch to new baudrate if new baudrate is supported
+		 */
+		baudrate = simple_strtoul(value, NULL, 10);
+
+		/* Not actually changing */
+		if (gd->baudrate == baudrate)
+			return 0;
+
+		for (i = 0; i < N_BAUDRATES; ++i) {
+			if (baudrate == baudrate_table[i])
+				break;
+		}
+		if (i == N_BAUDRATES) {
+			printf("## Baudrate %d bps not supported\n",
+				baudrate);
+			return 1;
+		}
+		printf("## Switch baudrate to %d"
+		       " bps and press ENTER ...\n", baudrate);
+		udelay(50000);
+		gd->baudrate = baudrate;
+#if defined(CONFIG_PPC) || defined(CONFIG_MCF52x2)
+		gd->bd->bi_baudrate = baudrate;
+#endif
+
+		serial_setbrg();
+		udelay(50000);
+		while (getc() != '\r')
+			;
+
+		return 0;
+	case env_op_delete:
+		printf("## Baudrate may not be deleted\n");
+		return 1;
+	default:
+		return 0;
+	}
+}
+U_BOOT_ENV_CALLBACK(baudrate, on_baudrate);
+
+#ifdef CONFIG_SERIAL_MULTI
 static struct serial_device *serial_devices;
 static struct serial_device *serial_current;
 
@@ -303,3 +360,4 @@ int uart_post_test(int flags)
 	return ret;
 }
 #endif
+#endif /* CONFIG_SERIAL_MULTI */
diff --git a/include/env_callback.h b/include/env_callback.h
index b450077..d399b39 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -36,7 +36,7 @@
  * a new assogiation in the ".callbacks" environment variable.
  */
 #define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
-	"loadaddr:loadaddr,bootfile:bootfile," \
+	"loadaddr:loadaddr,bootfile:bootfile,baudrate:baudrate," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 enum env_op {
-- 
1.7.11.5

[U-Boot] [RFC PATCH v2 5/5] env: Add a console env handler

[Joe Hershberger]

Remove the hard-coded console handler and use a callback instead


Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 common/cmd_nvedit.c    | 31 -------------------------------
 common/console.c       | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/env_callback.h |  1 +
 3 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 652e8ce..6a6421a 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -190,7 +190,6 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
 int _do_env_set(int flag, int argc, char * const argv[])
 {
 	int   i, len;
-	int   console = -1;
 	char  *name, *value, *s;
 	ENTRY e, *ep;
 
@@ -210,36 +209,6 @@ int _do_env_set(int flag, int argc, char * const argv[])
 	e.data = NULL;
 	hsearch_r(e, FIND, &ep, &env_htab);
 
-	/* Check for console redirection */
-	if (strcmp(name, "stdin") == 0)
-		console = stdin;
-	else if (strcmp(name, "stdout") == 0)
-		console = stdout;
-	else if (strcmp(name, "stderr") == 0)
-		console = stderr;
-
-	if (console != -1) {
-		if (argc < 3) {		/* Cannot delete it! */
-			printf("Can't delete \"%s\"\n", name);
-			return 1;
-		}
-
-#ifdef CONFIG_CONSOLE_MUX
-		i = iomux_doenv(console, argv[2]);
-		if (i)
-			return i;
-#else
-		/* Try assigning specified device */
-		if (console_assign(console, argv[2]) < 0)
-			return 1;
-
-#ifdef CONFIG_SERIAL_MULTI
-		if (serial_assign(argv[2]) < 0)
-			return 1;
-#endif
-#endif /* CONFIG_CONSOLE_MUX */
-	}
-
 	/*
 	 * Some variables like "ethaddr" and "serial#" can be set only
 	 * once and cannot be deleted; also, "ver" is readonly.
diff --git a/common/console.c b/common/console.c
index 1177f7d..18b4fcc 100644
--- a/common/console.c
+++ b/common/console.c
@@ -26,9 +26,55 @@
 #include <malloc.h>
 #include <stdio_dev.h>
 #include <exports.h>
+#include <environment.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static int on_console(const char *name, const char *value, enum env_op op)
+{
+	int console = -1;
+#ifdef CONFIG_CONSOLE_MUX
+	int retval;
+#endif
+
+	/* Check for console redirection */
+	if (strcmp(name, "stdin") == 0)
+		console = stdin;
+	else if (strcmp(name, "stdout") == 0)
+		console = stdout;
+	else if (strcmp(name, "stderr") == 0)
+		console = stderr;
+
+	switch (op) {
+	case env_op_create:
+	case env_op_overwrite:
+
+#ifdef CONFIG_CONSOLE_MUX
+		retval = iomux_doenv(console, value);
+		if (retval)
+			return retval;
+#else
+		/* Try assigning specified device */
+		if (console_assign(console, value) < 0)
+			return 1;
+
+#ifdef CONFIG_SERIAL_MULTI
+		if (serial_assign(value) < 0)
+			return 1;
+#endif
+#endif /* CONFIG_CONSOLE_MUX */
+		return 0;
+
+	case env_op_delete:
+		printf("Can't delete \"%s\"\n", name);
+		return 1;
+
+	default:
+		return 0;
+	}
+}
+U_BOOT_ENV_CALLBACK(console, on_console);
+
 #ifdef CONFIG_SYS_CONSOLE_IS_IN_ENV
 /*
  * if overwrite_console returns 1, the stdin, stderr and stdout
diff --git a/include/env_callback.h b/include/env_callback.h
index d399b39..0a50268 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -37,6 +37,7 @@
  */
 #define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
 	"loadaddr:loadaddr,bootfile:bootfile,baudrate:baudrate," \
+	"stdin:console,stdout:console,stderr:console," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 enum env_op {
-- 
1.7.11.5

[U-Boot] [RFC PATCH 2/5] env: Add a loadaddr env handler

[Joe Hershberger]

Remove the hard-coded loadaddr handler and use a callback instead

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 common/cmd_load.c      | 24 ++++++++++++++++++++++--
 common/cmd_nvedit.c    | 10 +---------
 include/env_callback.h |  1 +
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/common/cmd_load.c b/common/cmd_load.c
index f4d66de..8849596 100644
--- a/common/cmd_load.c
+++ b/common/cmd_load.c
@@ -30,9 +30,14 @@
 #include <net.h>
 #include <exports.h>
 #include <xyzModem.h>
+#include <environment.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+ulong load_addr = CONFIG_SYS_LOAD_ADDR;	/* Default Load Address */
+ulong save_addr;			/* Default Save Address */
+ulong save_size;			/* Default Save Size (in bytes) */
+
 #if defined(CONFIG_CMD_LOADB)
 static ulong load_serial_ymodem (ulong offset);
 #endif
@@ -50,6 +55,21 @@ static int do_echo = 1;
 
 /* -------------------------------------------------------------------- */
 
+static int on_loadaddr(const char *name, const char *value, enum env_op op)
+{
+	switch (op) {
+	case env_op_create:
+	case env_op_overwrite:
+		load_addr = simple_strtoul(value, NULL, 16);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+U_BOOT_ENV_CALLBACK(loadaddr, on_loadaddr);
+
 #if defined(CONFIG_CMD_LOADS)
 int do_load_serial (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
@@ -257,8 +277,8 @@ read_record (char *buf, ulong len)
 
 int do_save_serial (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	ulong offset = 0;
-	ulong size   = 0;
+	ulong offset = save_addr;
+	ulong size   = save_size;
 #ifdef	CONFIG_SYS_LOADS_BAUD_CHANGE
 	int save_baudrate, current_baudrate;
 
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index fd05e72..9e8ea12 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -79,10 +79,6 @@ SPI_FLASH|NVRAM|MMC|FAT|REMOTE} or CONFIG_ENV_IS_NOWHERE
  */
 #define	MAX_ENV_SIZE	(1 << 20)	/* 1 MiB */
 
-ulong load_addr = CONFIG_SYS_LOAD_ADDR;	/* Default Load Address */
-ulong save_addr;			/* Default Save Address */
-ulong save_size;			/* Default Save Size (in bytes) */
-
 /*
  * Table with supported baudrates (defined in config_xyz.h)
  */
@@ -341,12 +337,8 @@ int _do_env_set(int flag, int argc, char * const argv[])
 	 * Some variables should be updated when the corresponding
 	 * entry in the environment is changed
 	 */
-	if (strcmp(argv[1], "loadaddr") == 0) {
-		load_addr = simple_strtoul(argv[2], NULL, 16);
-		return 0;
-	}
 #if defined(CONFIG_CMD_NET)
-	else if (strcmp(argv[1], "bootfile") == 0) {
+	if (strcmp(argv[1], "bootfile") == 0) {
 		copy_filename(BootFile, argv[2], sizeof(BootFile));
 		return 0;
 	}
diff --git a/include/env_callback.h b/include/env_callback.h
index 4a92c24..31155e7 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -31,6 +31,7 @@
 #endif
 
 #define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
+	"loadaddr:loadaddr," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 enum env_op {
-- 
1.7.11.5

[U-Boot] [RFC PATCH 3/5] env: Add a bootfile env handler

[Joe Hershberger]

Remove the hard-coded bootfile handler and use a callback instead

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 common/cmd_nvedit.c    | 13 -------------
 include/env_callback.h |  2 +-
 net/net.c              | 16 ++++++++++++++++
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9e8ea12..ac15a3c 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -50,9 +50,6 @@
 #include <serial.h>
 #include <linux/stddef.h>
 #include <asm/byteorder.h>
-#if defined(CONFIG_CMD_NET)
-#include <net.h>
-#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -333,16 +330,6 @@ int _do_env_set(int flag, int argc, char * const argv[])
 		return 1;
 	}
 
-	/*
-	 * Some variables should be updated when the corresponding
-	 * entry in the environment is changed
-	 */
-#if defined(CONFIG_CMD_NET)
-	if (strcmp(argv[1], "bootfile") == 0) {
-		copy_filename(BootFile, argv[2], sizeof(BootFile));
-		return 0;
-	}
-#endif
 	return 0;
 }
 
diff --git a/include/env_callback.h b/include/env_callback.h
index 31155e7..42c3d91 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -31,7 +31,7 @@
 #endif
 
 #define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
-	"loadaddr:loadaddr," \
+	"loadaddr:loadaddr,bootfile:bootfile," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 enum env_op {
diff --git a/net/net.c b/net/net.c
index e8ff066..7d7196f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -82,6 +82,7 @@
 
 #include <common.h>
 #include <command.h>
+#include <environment.h>
 #include <net.h>
 #if defined(CONFIG_STATUS_LED)
 #include <miiphy.h>
@@ -208,6 +209,21 @@ static int NetTryCount;
 
 /**********************************************************************/
 
+static int on_bootfile(const char *name, const char *value, enum env_op op)
+{
+	switch (op) {
+	case env_op_create:
+	case env_op_overwrite:
+		copy_filename(BootFile, value, sizeof(BootFile));
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+U_BOOT_ENV_CALLBACK(bootfile, on_bootfile);
+
 /*
  * Check if autoload is enabled. If so, use either NFS or TFTP to download
  * the boot file.
-- 
1.7.11.5

[U-Boot] [RFC PATCH 4/5] env: Add a baudrate env handler

[Joe Hershberger]

Remove the hard-coded baudrate handler and use a callback instead

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 common/Makefile        |  2 +-
 common/cmd_nvedit.c    | 35 ------------------------------
 common/serial.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/env_callback.h |  2 +-
 4 files changed, 60 insertions(+), 37 deletions(-)

diff --git a/common/Makefile b/common/Makefile
index f61c9a1..1187960 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -32,7 +32,7 @@ COBJS-y += command.o
 COBJS-y += exports.o
 COBJS-$(CONFIG_SYS_HUSH_PARSER) += hush.o
 COBJS-y += s_record.o
-COBJS-$(CONFIG_SERIAL_MULTI) += serial.o
+COBJS-y += serial.o
 COBJS-y += xyzModem.o
 
 # core command
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index ac15a3c..652e8ce 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -47,7 +47,6 @@
 #include <errno.h>
 #include <malloc.h>
 #include <watchdog.h>
-#include <serial.h>
 #include <linux/stddef.h>
 #include <asm/byteorder.h>
 
@@ -77,12 +76,6 @@ SPI_FLASH|NVRAM|MMC|FAT|REMOTE} or CONFIG_ENV_IS_NOWHERE
 #define	MAX_ENV_SIZE	(1 << 20)	/* 1 MiB */
 
 /*
- * Table with supported baudrates (defined in config_xyz.h)
- */
-static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
-#define	N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
-
-/*
  * This variable is incremented on each do_env_set(), so it can
  * be used via get_env_id() as an indication, if the environment
  * has changed or not. So it is possible to reread an environment
@@ -263,34 +256,6 @@ int _do_env_set(int flag, int argc, char * const argv[])
 			return 1;
 		}
 #endif
-		/*
-		 * Switch to new baudrate if new baudrate is supported
-		 */
-		if (strcmp(name, "baudrate") == 0) {
-			int baudrate = simple_strtoul(argv[2], NULL, 10);
-			int i;
-			for (i = 0; i < N_BAUDRATES; ++i) {
-				if (baudrate == baudrate_table[i])
-					break;
-			}
-			if (i == N_BAUDRATES) {
-				printf("## Baudrate %d bps not supported\n",
-					baudrate);
-				return 1;
-			}
-			printf("## Switch baudrate to %d bps and"
-			       "press ENTER ...\n", baudrate);
-			udelay(50000);
-			gd->baudrate = baudrate;
-#if defined(CONFIG_PPC) || defined(CONFIG_MCF52x2)
-			gd->bd->bi_baudrate = baudrate;
-#endif
-
-			serial_setbrg();
-			udelay(50000);
-			while (getc() != '\r')
-				;
-		}
 	}
 
 	/* Delete only ? */
diff --git a/common/serial.c b/common/serial.c
index 75cc1bb..4ff1463 100644
--- a/common/serial.c
+++ b/common/serial.c
@@ -22,6 +22,7 @@
  */
 
 #include <common.h>
+#include <environment.h>
 #include <serial.h>
 #include <stdio_dev.h>
 #include <post.h>
@@ -29,6 +30,62 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/*
+ * Table with supported baudrates (defined in config_xyz.h)
+ */
+static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
+#define	N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
+
+static int on_baudrate(const char *name, const char *value, enum env_op op)
+{
+	int i;
+	int baudrate;
+
+	switch (op) {
+	case env_op_create:
+	case env_op_overwrite:
+		/*
+		 * Switch to new baudrate if new baudrate is supported
+		 */
+		baudrate = simple_strtoul(value, NULL, 10);
+
+		/* Not actually changing */
+		if (gd->baudrate == baudrate)
+			return 0;
+
+		for (i = 0; i < N_BAUDRATES; ++i) {
+			if (baudrate == baudrate_table[i])
+				break;
+		}
+		if (i == N_BAUDRATES) {
+			printf("## Baudrate %d bps not supported\n",
+				baudrate);
+			return 1;
+		}
+		printf("## Switch baudrate to %d"
+		       " bps and press ENTER ...\n", baudrate);
+		udelay(50000);
+		gd->baudrate = baudrate;
+#if defined(CONFIG_PPC) || defined(CONFIG_MCF52x2)
+		gd->bd->bi_baudrate = baudrate;
+#endif
+
+		serial_setbrg();
+		udelay(50000);
+		while (getc() != '\r')
+			;
+
+		return 0;
+	case env_op_delete:
+		printf("## Baudrate may not be deleted\n");
+		return 1;
+	default:
+		return 0;
+	}
+}
+U_BOOT_ENV_CALLBACK(baudrate, on_baudrate);
+
+#ifdef CONFIG_SERIAL_MULTI
 static struct serial_device *serial_devices;
 static struct serial_device *serial_current;
 
@@ -303,3 +360,4 @@ int uart_post_test(int flags)
 	return ret;
 }
 #endif
+#endif /* CONFIG_SERIAL_MULTI */
diff --git a/include/env_callback.h b/include/env_callback.h
index 42c3d91..3f39996 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -31,7 +31,7 @@
 #endif
 
 #define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
-	"loadaddr:loadaddr,bootfile:bootfile," \
+	"loadaddr:loadaddr,bootfile:bootfile,baudrate:baudrate," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 enum env_op {
-- 
1.7.11.5

[U-Boot] [RFC PATCH 5/5] env: Add a console env handler

[Joe Hershberger]

Remove the hard-coded console handler and use a callback instead


Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
 common/cmd_nvedit.c    | 31 -------------------------------
 common/console.c       | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/env_callback.h |  1 +
 3 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 652e8ce..6a6421a 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -190,7 +190,6 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
 int _do_env_set(int flag, int argc, char * const argv[])
 {
 	int   i, len;
-	int   console = -1;
 	char  *name, *value, *s;
 	ENTRY e, *ep;
 
@@ -210,36 +209,6 @@ int _do_env_set(int flag, int argc, char * const argv[])
 	e.data = NULL;
 	hsearch_r(e, FIND, &ep, &env_htab);
 
-	/* Check for console redirection */
-	if (strcmp(name, "stdin") == 0)
-		console = stdin;
-	else if (strcmp(name, "stdout") == 0)
-		console = stdout;
-	else if (strcmp(name, "stderr") == 0)
-		console = stderr;
-
-	if (console != -1) {
-		if (argc < 3) {		/* Cannot delete it! */
-			printf("Can't delete \"%s\"\n", name);
-			return 1;
-		}
-
-#ifdef CONFIG_CONSOLE_MUX
-		i = iomux_doenv(console, argv[2]);
-		if (i)
-			return i;
-#else
-		/* Try assigning specified device */
-		if (console_assign(console, argv[2]) < 0)
-			return 1;
-
-#ifdef CONFIG_SERIAL_MULTI
-		if (serial_assign(argv[2]) < 0)
-			return 1;
-#endif
-#endif /* CONFIG_CONSOLE_MUX */
-	}
-
 	/*
 	 * Some variables like "ethaddr" and "serial#" can be set only
 	 * once and cannot be deleted; also, "ver" is readonly.
diff --git a/common/console.c b/common/console.c
index 1177f7d..18b4fcc 100644
--- a/common/console.c
+++ b/common/console.c
@@ -26,9 +26,55 @@
 #include <malloc.h>
 #include <stdio_dev.h>
 #include <exports.h>
+#include <environment.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static int on_console(const char *name, const char *value, enum env_op op)
+{
+	int console = -1;
+#ifdef CONFIG_CONSOLE_MUX
+	int retval;
+#endif
+
+	/* Check for console redirection */
+	if (strcmp(name, "stdin") == 0)
+		console = stdin;
+	else if (strcmp(name, "stdout") == 0)
+		console = stdout;
+	else if (strcmp(name, "stderr") == 0)
+		console = stderr;
+
+	switch (op) {
+	case env_op_create:
+	case env_op_overwrite:
+
+#ifdef CONFIG_CONSOLE_MUX
+		retval = iomux_doenv(console, value);
+		if (retval)
+			return retval;
+#else
+		/* Try assigning specified device */
+		if (console_assign(console, value) < 0)
+			return 1;
+
+#ifdef CONFIG_SERIAL_MULTI
+		if (serial_assign(value) < 0)
+			return 1;
+#endif
+#endif /* CONFIG_CONSOLE_MUX */
+		return 0;
+
+	case env_op_delete:
+		printf("Can't delete \"%s\"\n", name);
+		return 1;
+
+	default:
+		return 0;
+	}
+}
+U_BOOT_ENV_CALLBACK(console, on_console);
+
 #ifdef CONFIG_SYS_CONSOLE_IS_IN_ENV
 /*
  * if overwrite_console returns 1, the stdin, stderr and stdout
diff --git a/include/env_callback.h b/include/env_callback.h
index 3f39996..94ffdad 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -32,6 +32,7 @@
 
 #define ENV_CALLBACK_LIST_STATIC ENV_CALLBACK_VAR ":callbacks," \
 	"loadaddr:loadaddr,bootfile:bootfile,baudrate:baudrate," \
+	"stdin:console,stdout:console,stderr:console," \
 	CONFIG_ENV_CALLBACK_LIST_STATIC
 
 enum env_op {
-- 
1.7.11.5