From patchwork Fri Aug 24 10:11:37 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gerlando Falauto X-Patchwork-Id: 179816 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 044262C00C4 for ; Fri, 24 Aug 2012 20:19:55 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 22D3828127; Fri, 24 Aug 2012 12:19:31 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GXmxGZjklIL5; Fri, 24 Aug 2012 12:19:30 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 4403828129; Fri, 24 Aug 2012 12:18:53 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id D55F328101 for ; Fri, 24 Aug 2012 12:18:42 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JhCdUeHJGt8R for ; Fri, 24 Aug 2012 12:18:40 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-de.keymile.com (mail-de.keymile.com [195.8.104.1]) by theia.denx.de (Postfix) with SMTP id AF5B0280C4 for ; Fri, 24 Aug 2012 12:18:38 +0200 (CEST) Received: from mailrelay.de.keymile.net ([10.9.1.54]) by eSafe SMTP Relay 1345792219; Fri, 24 Aug 2012 12:18:37 +0200 Received: from chber1-10555x.ch.keymile.net (chber1-10555x.ch.keymile.net [172.31.40.82]) by mailrelay.de.keymile.net (8.12.2/8.12.2) with ESMTP id q7OAHMjA026728; Fri, 24 Aug 2012 12:17:28 +0200 (MEST) From: Gerlando Falauto To: u-boot@lists.denx.de Date: Fri, 24 Aug 2012 12:11:37 +0200 Message-Id: <1345803102-21110-3-git-send-email-gerlando.falauto@keymile.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1345803102-21110-1-git-send-email-gerlando.falauto@keymile.com> References: <1321634955-5561-1-git-send-email-gerlando.falauto@keymile.com><1345803102-21110-1-git-send-email-gerlando.falauto@keymile.com> X-ESAFE-STATUS: [srvhellgate.de.keymile.net] Mail clean Cc: marex@denx.de, holger.brunck@keymile.com, Gerlando Falauto Subject: [U-Boot] [PATCH v4 2/7] env: unify logic to check and apply changes X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de 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. Signed-off-by: Gerlando Falauto Reviewed-by: Marek Vasut --- common/cmd_nvedit.c | 141 +++++++++++++++++++++++++++++++++------------------ include/search.h | 3 +- 2 files changed, 94 insertions(+), 50 deletions(-) diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index b8c7676..2f5dcbc 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -198,31 +198,19 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag, #endif /* - * Set a new environment variable, - * or replace or delete an existing one. + * Perform 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 instead. + * Returns 0 in case of success, 1 in case of failure. + * When (flag & H_FORCE) is set, do not print out any error message and 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) { - int i, len; 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,59 +221,75 @@ 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 - if (iomux_doenv(console, argv[2])) + if (iomux_doenv(console, newval)) return 1; #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 */ } /* - * Some variables like "ethaddr" and "serial#" can be set only - * once and cannot be deleted; also, "ver" is readonly. + * Some variables like "ethaddr" and "serial#" can be set only 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) @@ -299,6 +303,59 @@ 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, "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); @@ -336,20 +393,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(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/include/search.h b/include/search.h index ef53edb..a4a5ef4 100644 --- a/include/search.h +++ b/include/search.h @@ -99,6 +99,7 @@ extern int himport_r(struct hsearch_data *__htab, int __flag); /* Flags for himport_r() */ -#define H_NOCLEAR 1 /* do not clear hash table before importing */ +#define H_NOCLEAR (1 << 0) /* do not clear hash table before importing */ +#define H_FORCE (1 << 1) /* overwrite read-only/write-once variables */ #endif /* search.h */