[U-Boot] [PATCH RFC 0/4] Reworking of env

[U-Boot] [PATCH RFC 0/4] Reworking of env

[Gerlando Falauto]

Hi all,

trying to follow Wolfgang Denk's suggestion in
http://lists.denx.de/pipermail/u-boot/2011-September/102118.html:

>I agree that it makes sense to generalize and clean up this interface.
>It makes sense to select individual variables, and it makes sense to
>unify the "-f" handling to enforce actions on protected variables
>(while without "-f" only actions on the "normal" variables should be
>done).

>I can even imagine introducing a new variable that contains the name
>of the write-protected variables (and probably other properties, like
>being excluded from saveenv, etc.) - this has been discussed a number
>of times before, now we have the code base in place to actually
>implement it.

>All we need to do is extend the struct entry (in "include/search.h")
>by an "int flags"), and we can there register properties like
>read-only, don't-save etc.  In a first step this could be added
>transparently - so we could remove all the special handling of
>"ethaddr", "serial#" etc. in common/cmd_nvedit.c; then we could unify
>this to include "eth1addr" etc as well; then  we could extend it to
>read the names of such variables and their properties from a variable,
>etc.

here I am proposing a set of changes in the behaviour of the environment 
import/set_to_default functions.

Since a PATCH-er is worth a thousand words, I decided to take the risk 
and implement it first so to get your feedback on code rather than words...

=======
PATCH 1
=======
The most shocking change lies in the "new" himport_ex() function, which
has 3 new arguments:

1-2) "nvars", "vars", number and list of variables to take into account (0 means ALL)

3) "apply" callback function which is in charge of checking whether a 
variable can be overwritten, and possibly immediately apply the changes. 
This parameter would be either set to NULL (in which case nothing should 
change wrt to the past -- i.e. environment is blindly imported) or to 
"env_check_apply()" function, whose code was taken away from _do_env_set().
This would be useful, for instance, for "baudrate" or "stdin,stderr,stdout", 
whose changes would not otherwise be effective until the next reboot.

The idea is that there should be a single place where all the checks are
to be performed. So the same function env_check_apply() is called from 
_do_env_set() as well (thus keeping the previous behavior).

I know we're talking about heavy changes here... this is a just a draft -- 
I would just like some feedback about the whole idea, before putting more 
time into it.

=======
PATCH 2
=======
Here I added the same behaviour when deleting variables.
Not quite sure whether this makes sense or not.

=======
PATCH 3
=======
Here I added the code for setting to default individual variables

=======
PATCH 4
=======
Here I implemented the code for importing individual variables
(and also taking into account "-f" for forcing).

Thank you for your patience,
Gerlando Falauto

Gerlando Falauto (4):
  Groundwork for generalization of env interface
  env: check and apply changes on delete/destroy
  env: implement selective "env default"
  env: implement "env import -n var[,var...]"

 README                   |    2 +
 common/cmd_nvedit.c      |  221 +++++++++++++++++++++++++++++++--------------
 common/env_common.c      |   28 ++++++-
 include/config_cmd_all.h |    1 +
 include/environment.h    |    7 ++
 include/search.h         |   19 ++++-
 lib/hashtable.c          |   67 +++++++++++++--
 7 files changed, 266 insertions(+), 79 deletions(-)

[U-Boot] [PATCH RFC 1/4] Groundwork for generalization of env interface

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 common/cmd_nvedit.c   |  150 ++++++++++++++++++++++++++++++-------------------
 common/env_common.c   |   15 ++++-
 include/environment.h |    2 +
 include/search.h      |   13 ++++
 lib/hashtable.c       |   48 ++++++++++++++++
 5 files changed, 169 insertions(+), 59 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index e8b116d..05e365a 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -194,32 +194,23 @@ static int do_env_grep (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
 #endif
 
 /*
- * Set a new environment variable,
- * or replace or delete an existing one.
+ * Performs consistency checking before setting, replacing,
+ * or deleting an environment variable, then (if successful)
+ * apply the changes to internals so to make them effective.
+ * Code for this function was taken out of _do_env_set(),
+ * which now calls it.
+ * Also called as a callback function by himport_ex().
+ * Returns 0 in case of success, 1 in case of failure.
+ * When (flag & H_FORCE) is set, force overwriting of
+ * write-once variables.
  */
 
-int _do_env_set (int flag, int argc, char * const argv[])
+int env_check_apply(const char *name, const char *oldval,
+		    const char *newval, int flag)
 {
 	bd_t  *bd = gd->bd;
-	int   i, len;
+	int   i;
 	int   console = -1;
-	char  *name, *value, *s;
-	ENTRY e, *ep;
-
-	name = argv[1];
-
-	if (strchr(name, '=')) {
-		printf("## Error: illegal character '=' in variable name \"%s\"\n", name);
-		return 1;
-	}
-
-	env_id++;
-	/*
-	 * search if variable with this name already exists
-	 */
-	e.key = name;
-	e.data = NULL;
-	hsearch_r(e, FIND, &ep, &env_htab);
 
 	/* Check for console redirection */
 	if (strcmp(name, "stdin") == 0)
@@ -230,22 +221,22 @@ int _do_env_set (int flag, int argc, char * const argv[])
 		console = stderr;
 
 	if (console != -1) {
-		if (argc < 3) {		/* Cannot delete it! */
+		if ((newval == NULL) || (*newval == '\0')) {		/* Cannot delete it! */
 			printf("Can't delete \"%s\"\n", name);
 			return 1;
 		}
 
 #ifdef CONFIG_CONSOLE_MUX
-		i = iomux_doenv(console, argv[2]);
+		i = iomux_doenv(console, newval);
 		if (i)
 			return i;
 #else
 		/* Try assigning specified device */
-		if (console_assign(console, argv[2]) < 0)
+		if (console_assign(console, newval) < 0)
 			return 1;
 
 #ifdef CONFIG_SERIAL_MULTI
-		if (serial_assign(argv[2]) < 0)
+		if (serial_assign(newval) < 0)
 			return 1;
 #endif
 #endif /* CONFIG_CONSOLE_MUX */
@@ -255,23 +246,27 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	 * Some variables like "ethaddr" and "serial#" can be set only
 	 * once and cannot be deleted; also, "ver" is readonly.
 	 */
-	if (ep) {		/* variable exists */
+	if (oldval != NULL) { /* variable exists */
 #ifndef CONFIG_ENV_OVERWRITE
-		if ((strcmp(name, "serial#") == 0) ||
+		if ( ((flag & H_FORCE) == 0) &&
+		    ((strcmp(name, "serial#") == 0) ||
 		    ((strcmp(name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
-		     && (strcmp(ep->data, MK_STR(CONFIG_ETHADDR)) != 0)
+		     && (strcmp(oldval, MK_STR(CONFIG_ETHADDR)) != 0)
 #endif	/* CONFIG_OVERWRITE_ETHADDR_ONCE && CONFIG_ETHADDR */
 		    ) ) {
 			printf("Can't overwrite \"%s\"\n", name);
 			return 1;
 		}
 #endif
+	}
+	if ((oldval != NULL) /* variable exists */
+	|| ((flag & H_NOCLEAR) == 0)) { /* or we are in a scratched-out environment */
 		/*
 		 * Switch to new baudrate if new baudrate is supported
 		 */
 		if (strcmp(name, "baudrate") == 0) {
-			int baudrate = simple_strtoul(argv[2], NULL, 10);
+			int baudrate = simple_strtoul(newval, NULL, 10);
 			int i;
 			for (i = 0; i < N_BAUDRATES; ++i) {
 				if (baudrate == baudrate_table[i])
@@ -282,6 +277,10 @@ int _do_env_set (int flag, int argc, char * const argv[])
 					baudrate);
 				return 1;
 			}
+			if (gd->baudrate == baudrate) {
+				/* In case of unchanged baudrate, let's just say it's OK */
+				return 0;
+			}
 			printf ("## Switch baudrate to %d bps and press ENTER ...\n",
 				baudrate);
 			udelay(50000);
@@ -299,6 +298,71 @@ 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(name, "ipaddr") == 0) {
+		const char *s = newval;	/* always use only one arg */
+		char *e;
+		unsigned long addr;
+		bd->bi_ip_addr = 0;
+		for (addr = 0, i = 0; i < 4; ++i) {
+			ulong val = s ? simple_strtoul(s, &e, 10) : 0;
+			addr <<= 8;
+			addr  |= (val & 0xFF);
+			if (s) s = (*e) ? e+1 : e;
+		}
+		bd->bi_ip_addr = htonl(addr);
+		return 0;
+	} else if (strcmp(newval, "loadaddr") == 0) {
+		load_addr = simple_strtoul(newval, NULL, 16);
+		return 0;
+	}
+#if defined(CONFIG_CMD_NET)
+	else if (strcmp(newval, "bootfile") == 0) {
+		copy_filename(BootFile, newval, sizeof(BootFile));
+		return 0;
+	}
+#endif
+	return 0;
+}
+
+/*
+ * Set a new environment variable,
+ * or replace or delete an existing one.
+*/
+int _do_env_set (int flag, int argc, char * const argv[])
+{
+	int   i, len;
+	char  *name, *value, *s;
+	ENTRY e, *ep;
+
+	name = argv[1];
+	value = argv[2];
+
+	if (strchr(name, '=')) {
+		printf("## Error: illegal character '=' in variable name \"%s\"\n", name);
+		return 1;
+	}
+
+	env_id++;
+	/*
+	 * search if variable with this name already exists
+	 */
+	e.key = name;
+	e.data = NULL;
+	hsearch_r(e, FIND, &ep, &env_htab);
+
+	/* Perform requested checks. Notice how since we are overwriting
+	 * a single variable, we need to set H_NOCLEAR */
+	if (env_check_apply(name, ep ? ep->data : NULL, value, H_NOCLEAR))
+	{
+		debug("check function did not approve, refusing\n");
+		return 1;
+	}
+
 	/* Delete only ? */
 	if ((argc < 3) || argv[2] == NULL) {
 		int rc = hdelete_r(name, &env_htab);
@@ -336,34 +400,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 (strcmp(name, "ipaddr") == 0) {
-		char *s = argv[2];	/* always use only one arg */
-		char *e;
-		unsigned long addr;
-		bd->bi_ip_addr = 0;
-		for (addr = 0, i = 0; i < 4; ++i) {
-			ulong val = s ? simple_strtoul(s, &e, 10) : 0;
-			addr <<= 8;
-			addr  |= (val & 0xFF);
-			if (s) s = (*e) ? e+1 : e;
-		}
-		bd->bi_ip_addr = htonl(addr);
-		return 0;
-	} else 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) {
-		copy_filename(BootFile, argv[2], sizeof(BootFile));
-		return 0;
-	}
-#endif
 	return 0;
 }
 
diff --git a/common/env_common.c b/common/env_common.c
index 19149b5..7e1d824 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -172,6 +172,9 @@ const uchar *env_get_addr (int index)
 
 void set_default_env(const char *s)
 {
+	/* By default, do not apply changes as they will eventually
+	 * be applied by someone else */
+	apply_cb apply_function = NULL;
 	if (sizeof(default_environment) > ENV_SIZE) {
 		puts("*** Error - default environment is too large\n\n");
 		return;
@@ -183,14 +186,22 @@ void set_default_env(const char *s)
 				"using default environment\n\n",
 				s+1);
 		} else {
+			/* This set_to_default was explicitly asked for
+			 * by the user, as opposed to being a recovery
+			 * mechanism. Therefore we chack every single
+			 * variable and apply changes to the system
+			 * right away (e.g. baudrate, console).
+			 */
+			apply_function = env_check_apply;
 			puts(s);
 		}
 	} else {
 		puts("Using default environment\n\n");
 	}
 
-	if (himport_r(&env_htab, (char *)default_environment,
-		    sizeof(default_environment), '\0', 0) == 0) {
+	if (himport_ex(&env_htab, (char *)default_environment,
+		    sizeof(default_environment), '\0', 0,
+		      0, NULL, apply_function) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 	}
 	gd->flags |= GD_FLG_ENV_READY;
diff --git a/include/environment.h b/include/environment.h
index 6394a96..02c3488 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -174,6 +174,8 @@ void set_default_env(const char *s);
 /* Import from binary representation into hash table */
 int env_import(const char *buf, int check);
 
+int env_check_apply(const char *name, const char *oldval, const char *newval, int flag);
+
 #endif
 
 #endif	/* _ENVIRONMENT_H_ */
diff --git a/include/search.h b/include/search.h
index b4edd43..4d5b082 100644
--- a/include/search.h
+++ b/include/search.h
@@ -46,6 +46,11 @@ typedef struct entry {
 /* Opaque type for internal use.  */
 struct _ENTRY;
 
+/* Callback function to be called for checking whether the given change may
+ * be applied or not. Must return 0 for approval, 1 for denial. */
+typedef int (*apply_cb)(const char *name, const char *oldval,
+			const char *newval, int flag);
+
 /*
  * Family of hash table handling functions.  The functions also
  * have reentrant counterparts ending with _r.  The non-reentrant
@@ -97,7 +102,15 @@ extern int himport_r(struct hsearch_data *__htab,
 		     const char *__env, size_t __size, const char __sep,
 		     int __flag);
 
+extern int himport_ex(struct hsearch_data *__htab,
+		     const char *__env, size_t __size, const char __sep,
+		     int __flag,
+		     int __argc, char * const __argv[],
+		     apply_cb apply);
+
+
 /* Flags for himport_r() */
 #define	H_NOCLEAR	1	/* do not clear hash table before importing */
+#define H_FORCE		2	/* force overwriting of read-only or write-once variables */
 
 #endif /* search.h */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 026dbca..b088168 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -628,6 +628,34 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep,
 int himport_r(struct hsearch_data *htab,
 	      const char *env, size_t size, const char sep, int flag)
 {
+	return himport_ex(htab, env, size, sep, flag, 0, NULL, NULL);
+}
+
+/* Check whether variable name is amongst vars[] */
+static int process_var(const char *name, int nvars, char * const vars[])
+{
+	int i = 0;
+	/* No variables specified means process all of them*/
+	if (nvars == 0)
+		return 1;
+
+	for (i = 0; i < nvars; i++) {
+		if (!strcmp(name, vars[i])) {
+			return 1;
+		}
+	}
+	debug ("Skipping non-listed variable %s\n", name);
+	return 0;
+}
+
+
+
+int himport_ex(struct hsearch_data *htab,
+	      const char *env, size_t size, const char sep, int flag,
+	      int nvars, char * const vars[],
+	      int(*apply)(const char *, const char *, const char *, int)
+	      )
+{
 	char *data, *sp, *dp, *name, *value;
 
 	/* Test for correct arguments.  */
@@ -713,6 +741,8 @@ int himport_r(struct hsearch_data *htab,
 			*dp++ = '\0';	/* terminate name */
 
 			debug("DELETE CANDIDATE: \"%s\"\n", name);
+			if (!process_var(name, nvars, vars))
+				continue;
 
 			if (hdelete_r(name, htab) == 0)
 				debug("DELETE ERROR ##############################\n");
@@ -730,10 +760,28 @@ int himport_r(struct hsearch_data *htab,
 		*sp++ = '\0';	/* terminate value */
 		++dp;
 
+		/* Skip variables which are not supposed to be treated */
+		if (!process_var(name, nvars, vars))
+			continue;
+
 		/* enter into hash table */
 		e.key = name;
 		e.data = value;
 
+		/* if there is an apply function, check what it has to say */
+		if (apply != NULL)
+		{
+			debug("searching before calling cb function for variable %s\n", name);
+			/* Check for previous value so to provide it to apply callback */
+			hsearch_r(e, FIND, &rv, htab);
+			debug("previous value was %s\n", rv ? rv->data : "");
+			if (apply(name, rv ? rv->data : NULL, value, flag))
+			{
+				debug("callback function refused to set variable %s, skipping it!\n", name);
+				continue;
+			}
+		}
+
 		hsearch_r(e, ENTER, &rv, htab);
 		if (rv == NULL) {
 			printf("himport_r: can't insert \"%s=%s\" into hash table\n",
-- 
1.7.1

[U-Boot] [PATCH RFC 2/4] env: check and apply changes on delete/destroy

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 common/cmd_nvedit.c |    2 +-
 include/search.h    |    6 ++++--
 lib/hashtable.c     |   19 +++++++++++++------
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 05e365a..506a251 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -365,7 +365,7 @@ int _do_env_set (int flag, int argc, char * const argv[])
 
 	/* Delete only ? */
 	if ((argc < 3) || argv[2] == NULL) {
-		int rc = hdelete_r(name, &env_htab);
+		int rc = hdelete_r(name, &env_htab, NULL);
 		return !rc;
 	}
 
diff --git a/include/search.h b/include/search.h
index 4d5b082..cb30b67 100644
--- a/include/search.h
+++ b/include/search.h
@@ -68,7 +68,8 @@ struct hsearch_data {
 extern int hcreate_r(size_t __nel, struct hsearch_data *__htab);
 
 /* Destroy current internal hashing table.  */
-extern void hdestroy_r(struct hsearch_data *__htab);
+extern void hdestroy_r(struct hsearch_data *__htab,
+		       apply_cb apply);
 
 /*
  * Search for entry matching ITEM.key in internal hash table.  If
@@ -93,7 +94,8 @@ extern int hstrstr_r(const char *__match, int __last_idx, ENTRY ** __retval,
 		    struct hsearch_data *__htab);
 
 /* Search and delete entry matching ITEM.key in internal hash table. */
-extern int hdelete_r(const char *__key, struct hsearch_data *__htab);
+extern int hdelete_r(const char *__key, struct hsearch_data *__htab,
+		     apply_cb apply);
 
 extern ssize_t hexport_r(struct hsearch_data *__htab,
 		     const char __sep, char **__resp, size_t __size);
diff --git a/lib/hashtable.c b/lib/hashtable.c
index b088168..1b65b43 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -140,7 +140,8 @@ int hcreate_r(size_t nel, struct hsearch_data *htab)
  * be freed and the local static variable can be marked as not used.
  */
 
-void hdestroy_r(struct hsearch_data *htab)
+void hdestroy_r(struct hsearch_data *htab,
+		int(*apply)(const char *, const char *, const char *, int))
 {
 	int i;
 
@@ -154,7 +155,10 @@ void hdestroy_r(struct hsearch_data *htab)
 	for (i = 1; i <= htab->size; ++i) {
 		if (htab->table[i].used > 0) {
 			ENTRY *ep = &htab->table[i].entry;
-
+			if (apply != NULL) {
+				/* deletion is always forced */
+				apply(ep->key, ep->data, NULL, H_FORCE);
+			}
 			free((void *)ep->key);
 			free(ep->data);
 		}
@@ -399,7 +403,8 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
  * do that.
  */
 
-int hdelete_r(const char *key, struct hsearch_data *htab)
+int hdelete_r(const char *key, struct hsearch_data *htab, 	      
+	      int(*apply)(const char *, const char *, const char *, int))
 {
 	ENTRY e, *ep;
 	int idx;
@@ -415,7 +420,9 @@ int hdelete_r(const char *key, struct hsearch_data *htab)
 
 	/* free used ENTRY */
 	debug("hdelete: DELETING key \"%s\"\n", key);
-
+	if (apply != NULL) {
+		apply(ep->key, ep->data, NULL, H_FORCE);
+	}
 	free((void *)ep->key);
 	free(ep->data);
 	htab->table[idx].used = -1;
@@ -678,7 +685,7 @@ int himport_ex(struct hsearch_data *htab,
 		debug("Destroy Hash Table: %p table = %p\n", htab,
 		       htab->table);
 		if (htab->table)
-			hdestroy_r(htab);
+			hdestroy_r(htab, apply);
 	}
 
 	/*
@@ -744,7 +751,7 @@ int himport_ex(struct hsearch_data *htab,
 			if (!process_var(name, nvars, vars))
 				continue;
 
-			if (hdelete_r(name, htab) == 0)
+			if (hdelete_r(name, htab, apply) == 0)
 				debug("DELETE ERROR ##############################\n");
 
 			continue;
-- 
1.7.1

[U-Boot] [PATCH RFC 3/4] env: implement selective "env default"

[Gerlando Falauto]

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 README                   |    2 ++
 common/cmd_nvedit.c      |   38 ++++++++++++++++++++++++++++++++------
 common/env_common.c      |   13 +++++++++++++
 include/config_cmd_all.h |    1 +
 include/environment.h    |    5 +++++
 5 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/README b/README
index a43da97..4b28854 100644
--- a/README
+++ b/README
@@ -705,6 +705,8 @@ The following options need to be configured:
 		CONFIG_CMD_CONSOLE	  coninfo
 		CONFIG_CMD_CRC32	* crc32
 		CONFIG_CMD_DATE		* support for RTC, date/time...
+		CONFIG_CMD_DEFAULTENV_VARS
+					* Reset individual variables to default
 		CONFIG_CMD_DHCP		* DHCP support
 		CONFIG_CMD_DIAG		* Diagnostics
 		CONFIG_CMD_DS4510	* ds4510 I2C gpio commands
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 506a251..8636004 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -615,13 +615,36 @@ int envmatch(uchar *s1, int i2)
 	return -1;
 }
 
-static int do_env_default(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_env_default(cmd_tbl_t *cmdtp, int __flag, int argc, char * const argv[])
 {
-	if ((argc != 2) || (strcmp(argv[1], "-f") != 0))
-		return cmd_usage(cmdtp);
-
-	set_default_env("## Resetting to default environment\n");
-	return 0;
+	int all = 0, flag = 0;
+	debug("Initial value for argc=%d\n", argc);
+	while (--argc > 0 && **++argv == '-') {
+		char *arg = *argv;
+		while (*++arg) {
+			switch (*arg) {
+			case 'a':		/* default all */
+				all = 1;
+				break;
+			case 'f':		/* force */
+				flag |= H_FORCE;
+				break;
+			default:
+				return cmd_usage(cmdtp);
+			}
+		}
+	}
+	debug("Final value for argc=%d\n", argc);
+	if (all && (argc == 0)) {
+		/* Reset the whole environment */
+		set_default_env("## Resetting to default environment\n");
+		return 0;
+	} else if (!all && (argc > 0)) {
+		/* Reset individual variables */
+		env_default_vars(argc, argv);
+		return 0;
+	}
+	return cmd_usage(cmdtp);
 }
 
 static int do_env_delete(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
@@ -947,6 +970,9 @@ U_BOOT_CMD(
 	"ask name [message] [size] - ask for environment variable\nenv "
 #endif
 	"default -f - reset default environment\n"
+#if defined(CONFIG_CMD_DEFAULTENV_VARS)
+	"env default var [...] - reset variable(s) to their default value\n"
+#endif
 #if defined(CONFIG_CMD_EDITENV)
 	"env edit name - edit environment variable\n"
 #endif
diff --git a/common/env_common.c b/common/env_common.c
index 7e1d824..c2769a8 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -207,6 +207,19 @@ void set_default_env(const char *s)
 	gd->flags |= GD_FLG_ENV_READY;
 }
 
+#ifdef CONFIG_CMD_DEFAULTENV_VARS
+
+/* [re]set individual variables to their value in the default environment */
+int env_default_vars(int nvars, char * const vars[])
+{
+	/* Special use-case: import from default environment
+	   (and use \0 as a separator) */
+	return himport_ex(&env_htab, (const char *)default_environment, sizeof(default_environment), '\0', H_NOCLEAR,
+			 nvars, vars, env_check_apply);
+}
+
+#endif /* CONFIG_CMD_DEFAULTENV_VARS */
+
 /*
  * Check if CRC is valid and (if yes) import the environment.
  * Note that "buf" may or may not be aligned.
diff --git a/include/config_cmd_all.h b/include/config_cmd_all.h
index 9716f9c..e728eae 100644
--- a/include/config_cmd_all.h
+++ b/include/config_cmd_all.h
@@ -25,6 +25,7 @@
 #define CONFIG_CMD_CDP		/* Cisco Discovery Protocol	*/
 #define CONFIG_CMD_CONSOLE	/* coninfo			*/
 #define CONFIG_CMD_DATE		/* support for RTC, date/time...*/
+#define CONFIG_CMD_DEFAULTENV_VARS /* default individ variables */
 #define CONFIG_CMD_DHCP		/* DHCP Support			*/
 #define CONFIG_CMD_DIAG		/* Diagnostics			*/
 #define CONFIG_CMD_DISPLAY	/* Display support		*/
diff --git a/include/environment.h b/include/environment.h
index 02c3488..7555c3b 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -171,6 +171,11 @@ void env_crc_update (void);
 /* [re]set to the default environment */
 void set_default_env(const char *s);
 
+#ifdef CONFIG_CMD_DEFAULTENV_VARS
+/* [re]set individual variables to their value in the default environment */
+int env_default_vars(int nvars, char * const vars[]);
+#endif
+
 /* Import from binary representation into hash table */
 int env_import(const char *buf, int check);
 
-- 
1.7.1

[U-Boot] [PATCH RFC 4/4] env: implement "env import -n var[, var...]"

[Gerlando Falauto]

Implemented selective importing of variables in
env import

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 common/cmd_nvedit.c |   31 ++++++++++++++++++++++++++-----
 1 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 8636004..757a088 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -804,15 +804,18 @@ sep_err:
  *	size:	length of input data; if missing, proper '\0'
  *		termination is mandatory
  */
-static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+#define MAX_NVARS 16
+static int do_env_import(cmd_tbl_t *cmdtp, int __flag, int argc, char * const argv[])
 {
 	char	*cmd, *addr;
 	char	sep = '\n';
 	int	chk = 0;
 	int	fmt = 0;
-	int	del = 0;
+	int	flag = H_NOCLEAR;
 	size_t	size;
-
+	char	*vars[MAX_NVARS];
+	int	nvars = 0;
+	char 	*s;
 	cmd = *argv;
 
 	while (--argc > 0 && **++argv == '-') {
@@ -836,7 +839,24 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 				sep = '\n';
 				break;
 			case 'd':
-				del = 1;
+				flag &= ~H_NOCLEAR;
+				break;
+			case 'f':
+				flag |= H_FORCE;
+				break;
+			case 'n':
+				s = *++argv;
+				argc--;
+				do {
+					if (nvars >= MAX_NVARS)
+					{
+						printf("No more than %d variables can be imported at a single time\n", MAX_NVARS);
+						return 1;
+					}
+					vars[nvars++] = strsep(&s, ",");
+					debug("Considering variable %s\n", vars[nvars-1]);
+				} while (s);
+				debug("Importing a total of %d variables\n", nvars);
 				break;
 			default:
 				return cmd_usage(cmdtp);
@@ -887,7 +907,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		addr = (char *)ep->data;
 	}
 
-	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR) == 0) {
+	if (himport_ex(&env_htab, addr, size, sep, flag, 
+		       nvars, vars, NULL) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 		return 1;
 	}
-- 
1.7.1

[U-Boot] [PATCH RFC 4/4] env: implement "env import -n var[, var...]"

[Mike Frysinger]

On Friday, September 30, 2011 08:04:58 Gerlando Falauto wrote:
> +					if (nvars >= MAX_NVARS)
> +					{

that brace needs to be cuddled
-mike

[U-Boot] [PATCH RFC 2/4] env: check and apply changes on delete/destroy

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1317384298-27025-3-git-send-email-gerlando.falauto@keymile.com> you wrote:
> 
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
>  common/cmd_nvedit.c |    2 +-
>  include/search.h    |    6 ++++--
>  lib/hashtable.c     |   19 +++++++++++++------
>  3 files changed, 18 insertions(+), 9 deletions(-)

Checkpatch says:

total: 1 errors, 2 warnings, 81 lines checked

Please clean up and resubmit.  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: wd at denx.de
"What terrible way to die."
"There are no good ways."
	-- Sulu and Kirk, "That Which Survives", stardate unknown

[U-Boot] [PATCH RFC 1/4] Groundwork for generalization of env interface

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1317384298-27025-2-git-send-email-gerlando.falauto@keymile.com> you wrote:
> 
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
>  common/cmd_nvedit.c   |  150 ++++++++++++++++++++++++++++++-------------------
>  common/env_common.c   |   15 ++++-
>  include/environment.h |    2 +
>  include/search.h      |   13 ++++
>  lib/hashtable.c       |   48 ++++++++++++++++
>  5 files changed, 169 insertions(+), 59 deletions(-)

Checkpatch says:

total: 5 errors, 15 warnings, 353 lines checked

Please clean up and resubmit.  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: wd at denx.de
He'd heard her use that sweet, innocent  tone  of  voice  before.  It
meant that, pretty soon, there was going to be trouble.
                                        - Terry Pratchett, _Truckers_