Patchwork [U-Boot,v1,3/5] env: unify logic to check and apply changes

login
register
mail settings
Submitter Gerlando Falauto
Date Nov. 18, 2011, 4:49 p.m.
Message ID <1321634955-5561-4-git-send-email-gerlando.falauto@keymile.com>
Download mbox | patch
Permalink /patch/126452/
State Superseded
Delegated to: Wolfgang Denk
Headers show

Comments

Gerlando Falauto - Nov. 18, 2011, 4:49 p.m.
The logic of checking special parameters (e.g. baudrate, stdin, stdout,
for a valid value and/or whether can be overwritten) and applying the
new value to the running system is now all within a single function
env_check_apply() which can be called whenever changes are made
to the environment, no matter if by set, default or import.

With this patch env_check_apply() is only called by "env set",
retaining previous behavior.

Also allow for selectively importing/resetting variables.

So add 3 new arguments to himport_r():
o "nvars", "vars":, number and list of variables to take into account
   (0 means ALL)

o "apply" callback function to check whether a variable can be
  overwritten, and possibly immediately apply the changes;
  when NULL, no check is performed.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 common/cmd_nvedit.c   |  163 +++++++++++++++++++++++++++++++-----------------
 common/env_common.c   |    6 +-
 include/environment.h |    7 ++
 include/search.h      |   17 +++++-
 lib/hashtable.c       |   43 +++++++++++++-
 5 files changed, 174 insertions(+), 62 deletions(-)
Simon Glass - Dec. 7, 2011, 1:50 a.m.
Hi,

On Fri, Nov 18, 2011 at 8:49 AM, Gerlando Falauto
<gerlando.falauto@keymile.com> wrote:
> The logic of checking special parameters (e.g. baudrate, stdin, stdout,
> for a valid value and/or whether can be overwritten) and applying the
> new value to the running system is now all within a single function
> env_check_apply() which can be called whenever changes are made
> to the environment, no matter if by set, default or import.
>
> With this patch env_check_apply() is only called by "env set",
> retaining previous behavior.
>
> Also allow for selectively importing/resetting variables.
>
> So add 3 new arguments to himport_r():
> o "nvars", "vars":, number and list of variables to take into account
>   (0 means ALL)
>
> o "apply" callback function to check whether a variable can be
>  overwritten, and possibly immediately apply the changes;
>  when NULL, no check is performed.
>
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>

I haven't seen any comments on this series, but it seems good to me.
However it no longer applies - can you please rebase and resend?

Regards,
Simon

> ---
>  common/cmd_nvedit.c   |  163 +++++++++++++++++++++++++++++++-----------------
>  common/env_common.c   |    6 +-
>  include/environment.h |    7 ++
>  include/search.h      |   17 +++++-
>  lib/hashtable.c       |   43 +++++++++++++-
>  5 files changed, 174 insertions(+), 62 deletions(-)
>
> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> index 7194ade..d6ea25d 100644
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
> @@ -196,32 +196,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_r().
> + * 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)
> @@ -232,22 +223,23 @@ 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')) {
> +                       /* We cannot delete stdin/stdout/stderr */
>                        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,25 +247,35 @@ 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.
> +        * once and cannot be deleted, unless CONFIG_ENV_OVERWRITE
> +        * is defined.
>         */
> -       if (ep) {               /* variable exists */
>  #ifndef CONFIG_ENV_OVERWRITE
> +       if (oldval != NULL &&                   /* variable exists */
> +               (flag & H_FORCE) == 0) {        /* and we are not forced */
>                if ((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
> +       /*
> +        * When we change baudrate, or we are doing an env default -a
> +        * (which will erase all variables prior to calling this),
> +        * we want the baudrate to actually change - for real.
> +        */
> +       if (oldval != NULL ||                   /* variable exists */
> +               (flag & H_NOCLEAR) == 0) {      /* or env is clear */
>                /*
>                 * 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])
> @@ -284,6 +286,10 @@ int _do_env_set (int flag, int argc, char * const argv[])
>                                        baudrate);
>                                return 1;
>                        }
> +                       if (gd->baudrate == baudrate) {
> +                               /* If unchanged, we just say it's OK */
> +                               return 0;
> +                       }
>                        printf ("## Switch baudrate to %d bps and press ENTER ...\n",
>                                baudrate);
>                        udelay(50000);
> @@ -301,6 +307,74 @@ 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;
> +               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);
> @@ -338,34 +412,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;
>  }
>
> @@ -888,7 +934,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_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
> +                       0, NULL, NULL) == 0) {
>                error("Environment import failed: errno = %d\n", errno);
>                return 1;
>        }
> diff --git a/common/env_common.c b/common/env_common.c
> index c7e9bea..9d8c59e 100644
> --- a/common/env_common.c
> +++ b/common/env_common.c
> @@ -190,7 +190,8 @@ void set_default_env(const char *s)
>        }
>
>        if (himport_r(&env_htab, (char *)default_environment,
> -                   sizeof(default_environment), '\0', 0) == 0) {
> +                       sizeof(default_environment), '\0', 0,
> +                       0, NULL, NULL) == 0) {
>                error("Environment import failed: errno = %d\n", errno);
>        }
>        gd->flags |= GD_FLG_ENV_READY;
> @@ -215,7 +216,8 @@ int env_import(const char *buf, int check)
>                }
>        }
>
> -       if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0)) {
> +       if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
> +                       0, NULL, NULL)) {
>                gd->flags |= GD_FLG_ENV_READY;
>                return 1;
>        }
> diff --git a/include/environment.h b/include/environment.h
> index 6394a96..1936411 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -174,6 +174,13 @@ void set_default_env(const char *s);
>  /* Import from binary representation into hash table */
>  int env_import(const char *buf, int check);
>
> +/*
> + * Check if variable "name" can be changed from oldval to newval,
> + * and if so, apply the changes (e.g. baudrate)
> + */
> +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 ef53edb..2a59e03 100644
> --- a/include/search.h
> +++ b/include/search.h
> @@ -47,6 +47,13 @@ typedef struct entry {
>  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
>  * functions all work on a signle internal hashing table.
> @@ -94,11 +101,19 @@ extern ssize_t hexport_r(struct hsearch_data *__htab,
>                     const char __sep, char **__resp, size_t __size,
>                     int argc, char * const argv[]);
>
> +/*
> + * nvars, vars: variables to import (nvars == 0 means all)
> + * apply_cb: callback function to check validity of the new argument,
> + * and possibly apply changes (NULL means accept everything)
> + */
>  extern int himport_r(struct hsearch_data *__htab,
>                     const char *__env, size_t __size, const char __sep,
> -                    int __flag);
> +                    int __flag,
> +                    int nvars, char * const vars[],
> +                    apply_cb apply);
>
>  /* Flags for himport_r() */
>  #define        H_NOCLEAR       1       /* do not clear hash table before importing */
> +#define H_FORCE                2       /* overwrite read-only/write-once variables */
>
>  #endif /* search.h */
> diff --git a/lib/hashtable.c b/lib/hashtable.c
> index b7ba341..22421f9 100644
> --- a/lib/hashtable.c
> +++ b/lib/hashtable.c
> @@ -603,6 +603,22 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep,
>  * himport()
>  */
>
> +/* 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;
> +}
> +
>  /*
>  * Import linearized data into hash table.
>  *
> @@ -639,7 +655,9 @@ 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)
> +               const char *env, size_t size, const char sep, int flag,
> +               int nvars, char * const vars[],
> +               apply_cb apply)
>  {
>        char *data, *sp, *dp, *name, *value;
>
> @@ -726,6 +744,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");
> @@ -743,10 +763,31 @@ 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  %s\n", name);
> +                       /*
> +                        * Search for variable in existing env, so to pass
> +                        * its previous value to the 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 mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 7194ade..d6ea25d 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -196,32 +196,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_r().
+ * 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)
@@ -232,22 +223,23 @@  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')) {
+			/* We cannot delete stdin/stdout/stderr */
 			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,25 +247,35 @@  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.
+	 * once and cannot be deleted, unless CONFIG_ENV_OVERWRITE
+	 * is defined.
 	 */
-	if (ep) {		/* variable exists */
 #ifndef CONFIG_ENV_OVERWRITE
+	if (oldval != NULL &&			/* variable exists */
+		(flag & H_FORCE) == 0) {	/* and we are not forced */
 		if ((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
+	/*
+	 * When we change baudrate, or we are doing an env default -a
+	 * (which will erase all variables prior to calling this),
+	 * we want the baudrate to actually change - for real.
+	 */
+	if (oldval != NULL ||			/* variable exists */
+		(flag & H_NOCLEAR) == 0) {	/* or env is clear */
 		/*
 		 * 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])
@@ -284,6 +286,10 @@  int _do_env_set (int flag, int argc, char * const argv[])
 					baudrate);
 				return 1;
 			}
+			if (gd->baudrate == baudrate) {
+				/* If unchanged, we just say it's OK */
+				return 0;
+			}
 			printf ("## Switch baudrate to %d bps and press ENTER ...\n",
 				baudrate);
 			udelay(50000);
@@ -301,6 +307,74 @@  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;
+		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);
@@ -338,34 +412,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;
 }
 
@@ -888,7 +934,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_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
+			0, NULL, NULL) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 		return 1;
 	}
diff --git a/common/env_common.c b/common/env_common.c
index c7e9bea..9d8c59e 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -190,7 +190,8 @@  void set_default_env(const char *s)
 	}
 
 	if (himport_r(&env_htab, (char *)default_environment,
-		    sizeof(default_environment), '\0', 0) == 0) {
+			sizeof(default_environment), '\0', 0,
+			0, NULL, NULL) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 	}
 	gd->flags |= GD_FLG_ENV_READY;
@@ -215,7 +216,8 @@  int env_import(const char *buf, int check)
 		}
 	}
 
-	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0)) {
+	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
+			0, NULL, NULL)) {
 		gd->flags |= GD_FLG_ENV_READY;
 		return 1;
 	}
diff --git a/include/environment.h b/include/environment.h
index 6394a96..1936411 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -174,6 +174,13 @@  void set_default_env(const char *s);
 /* Import from binary representation into hash table */
 int env_import(const char *buf, int check);
 
+/*
+ * Check if variable "name" can be changed from oldval to newval,
+ * and if so, apply the changes (e.g. baudrate)
+ */
+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 ef53edb..2a59e03 100644
--- a/include/search.h
+++ b/include/search.h
@@ -47,6 +47,13 @@  typedef struct entry {
 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
  * functions all work on a signle internal hashing table.
@@ -94,11 +101,19 @@  extern ssize_t hexport_r(struct hsearch_data *__htab,
 		     const char __sep, char **__resp, size_t __size,
 		     int argc, char * const argv[]);
 
+/*
+ * nvars, vars: variables to import (nvars == 0 means all)
+ * apply_cb: callback function to check validity of the new argument,
+ * and possibly apply changes (NULL means accept everything)
+ */
 extern int himport_r(struct hsearch_data *__htab,
 		     const char *__env, size_t __size, const char __sep,
-		     int __flag);
+		     int __flag,
+		     int nvars, char * const vars[],
+		     apply_cb apply);
 
 /* Flags for himport_r() */
 #define	H_NOCLEAR	1	/* do not clear hash table before importing */
+#define H_FORCE		2	/* overwrite read-only/write-once variables */
 
 #endif /* search.h */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index b7ba341..22421f9 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -603,6 +603,22 @@  ssize_t hexport_r(struct hsearch_data *htab, const char sep,
  * himport()
  */
 
+/* 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;
+}
+
 /*
  * Import linearized data into hash table.
  *
@@ -639,7 +655,9 @@  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)
+		const char *env, size_t size, const char sep, int flag,
+		int nvars, char * const vars[],
+		apply_cb apply)
 {
 	char *data, *sp, *dp, *name, *value;
 
@@ -726,6 +744,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");
@@ -743,10 +763,31 @@  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  %s\n", name);
+			/*
+			 * Search for variable in existing env, so to pass
+			 * its previous value to the 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",