From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gerlando Falauto Date: Fri, 30 Mar 2012 15:00:41 +0200 Subject: [U-Boot] [PATCH v2 3/3] env: make "env default" selective, check and apply In-Reply-To: <201203292225.33982.marex@denx.de> References: <1321634955-5561-1-git-send-email-gerlando.falauto@keymile.com> <1323264605-13541-4-git-send-email-gerlando.falauto@keymile.com> <201203292225.33982.marex@denx.de> Message-ID: <4F75AE79.7020500@keymile.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 03/29/2012 10:25 PM, Marek Vasut wrote: > Dear Gerlando Falauto, > >> Changes in the syntax (user API) for "env default": >> -f: override write-once variables >> -a: all (resetting the whole env is NOT the default behavior) >> >> Signed-off-by: Gerlando Falauto > > I have to admit I'm not much of a fan of how you use this apply() callback, is > it really necessary? See my previous email. > Also, do we need special command for default env? I am sorry, I don't get the question. What do you mean? It's always the same "env import" with an augmented command line. If you're referring to CONFIG_CMD_DEFAULTENV_VARS, I don't think it's neeed either, but it was requested at some previous time. I also think it's misleading to have some syntax for the same command conditionally enabled. Has it ever been done before? Thank you, Gerlando > M > >> --- >> README | 2 ++ >> common/cmd_nvedit.c | 43 >> ++++++++++++++++++++++++++++++++++++------- common/env_common.c | >> 31 ++++++++++++++++++++++++++++++- >> include/config_cmd_all.h | 1 + >> include/environment.h | 5 +++++ >> 5 files changed, 74 insertions(+), 8 deletions(-) >> >> diff --git a/README b/README >> index fda0190..e924575 100644 >> --- a/README >> +++ b/README >> @@ -724,6 +724,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 871b3b1..317bd1c 100644 >> --- a/common/cmd_nvedit.c >> +++ b/common/cmd_nvedit.c >> @@ -674,14 +674,40 @@ int envmatch(uchar *s1, int i2) >> return -1; >> } >> >> -static int do_env_default(cmd_tbl_t *cmdtp, int flag, >> +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; >> + } >> +#ifdef CONFIG_CMD_DEFAULTENV_VARS >> + if (!all&& (argc> 0)) { >> + /* Reset individual variables */ >> + env_default_vars(argc, argv); >> + return 0; >> + } >> +#endif >> + return cmd_usage(cmdtp); >> } >> >> static int do_env_delete(cmd_tbl_t *cmdtp, int flag, >> @@ -1012,7 +1038,10 @@ U_BOOT_CMD( >> #if defined(CONFIG_CMD_ASKENV) >> "ask name [message] [size] - ask for environment variable\nenv " >> #endif >> - "default -f - reset default environment\n" >> + "default [-f] -a - [forcibly] reset default environment\n" >> +#if defined(CONFIG_CMD_DEFAULTENV_VARS) >> + "env default [-f] var [...] - [forcibly] reset variable(s) to their >> default values\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 7e2bb2f..56719a6 100644 >> --- a/common/env_common.c >> +++ b/common/env_common.c >> @@ -157,6 +157,11 @@ 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; >> @@ -168,6 +173,14 @@ 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 { >> @@ -176,12 +189,28 @@ void set_default_env(const char *s) >> >> if (himport_r(&env_htab, (char *)default_environment, >> sizeof(default_environment), '\0', 0, >> - 0, NULL, NULL) == 0) >> + 0, NULL, apply_function) == 0) >> error("Environment import failed: errno = %d\n", errno); >> >> 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_r(&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 3a3e6b8..8f0d4db 100644 >> --- a/include/environment.h >> +++ b/include/environment.h >> @@ -190,6 +190,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);