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.fala...@keymile.com> --- common/cmd_nvedit.c | 174 +++++++++++++++++++++++++++++++------------------ common/env_common.c | 6 +- include/environment.h | 7 ++ include/search.h | 17 +++++- lib/hashtable.c | 43 ++++++++++++- 5 files changed, 180 insertions(+), 67 deletions(-) diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 5995354..a31d413 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -197,32 +197,23 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag, #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) @@ -233,22 +224,24 @@ int _do_env_set(int flag, int argc, char * const argv[]) console = stderr; if (console != -1) { - if (argc < 3) { /* Cannot delete it! */ - printf("Can't delete \"%s\"\n", name); + if ((newval == NULL) || (*newval == '\0')) { + /* We cannot delete stdin/stdout/stderr */ + if ((flag & H_FORCE) == 0) + 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 */ @@ -256,37 +249,52 @@ 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]) break; } if (i == N_BAUDRATES) { - printf("## Baudrate %d bps not supported\n", - baudrate); + if ((flag & H_FORCE) == 0) + printf("## Baudrate %d bps not " + "supported\n", 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); + "press ENTER ...\n", baudrate); udelay(50000); gd->baudrate = baudrate; #if defined(CONFIG_PPC) || defined(CONFIG_MCF52x2) @@ -300,6 +308,73 @@ 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(name, "loadaddr") == 0) { + load_addr = simple_strtoul(newval, NULL, 16); + return 0; + } +#if defined(CONFIG_CMD_NET) + else if (strcmp(name, "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); @@ -337,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; } @@ -886,7 +933,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, 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 8a71096..7e2bb2f 100644 --- a/common/env_common.c +++ b/common/env_common.c @@ -175,7 +175,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; @@ -200,7 +201,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 3c145af..3a3e6b8 100644 --- a/include/environment.h +++ b/include/environment.h @@ -193,6 +193,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 /* DO_DEPS_ONLY */ #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 abd61c8..75b9b07 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