[U-Boot] [PATCH v4 6/7] env: make "env default" selective, check and apply

Gerlando Falauto
2012-08-24T15:10:14Z

On 08/24/2012 04:56 PM, Marek Vasut wrote:
> Dear Gerlando Falauto,
>
>> Change the syntax (user API) for "env default":
>> -f: override write-once variables
>> var... : accept individual variable(s)
>> -a: all (resetting the whole env is NOT the default behavior)
>>
>> Enable variable checking and make changes effective by
>> enabling do_apply argument to himport_r().
>>
>> Signed-off-by: Gerlando Falauto<gerlando.falauto@keymile.com>
>> ---
>> common/cmd_nvedit.c | 40 ++++++++++++++++++++++++++++++++++------
>> common/env_common.c | 28 +++++++++++++++++++++++++++-
>> include/environment.h | 3 +++
>> 3 files changed, 64 insertions(+), 7 deletions(-)
>>
>> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
>> index b0860f3..ac2b985 100644
>> --- a/common/cmd_nvedit.c
>> +++ b/common/cmd_nvedit.c
>> @@ -656,14 +656,41 @@ 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_RET_USAGE;
>> + int all = 0, flag = 0;
>>
>> - set_default_env("## Resetting to default environment\n");
>> - return 0;
>> + debug("Initial value for argc=%d\n", argc);
>> + while (--argc> 0&& **++argv == '-') {
>
> mmmmm ... **++argv, yummy :) This might use some cleanup, to make more readable.

Uhm, this pattern is being used all over the place on that file (that's
where I copied it from).

> Don't we have some getopt or something too ?

Not that I (or "git grep") know of.

>
>> + 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;
>> + }
>> + if (!all&& (argc> 0)) {
>> + /* Reset individual variables */
>> + set_default_vars(argc, argv);
>> + return 0;
>> + }
>> +
>> + return cmd_usage(cmdtp);
>> }
>>
>> static int do_env_delete(cmd_tbl_t *cmdtp, int flag,
>> @@ -994,7 +1021,8 @@ 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"
>> + "env default [-f] var [...] - [forcibly] reset variable(s) to their
>> default values\n" #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 c6e7c4c..482d715 100644
>> --- a/common/env_common.c
>> +++ b/common/env_common.c
>> @@ -177,6 +177,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
>> + */
>> + int do_apply = 0;
>> if (sizeof(default_environment)> ENV_SIZE) {
>> puts("*** Error - default environment is too large\n\n");
>> return;
>> @@ -188,6 +193,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 check every single
>> + * variable and apply changes to the system
>> + * right away (e.g. baudrate, console).
>> + */
>> + do_apply = 1;
>> puts(s);
>> }
>> } else {
>> @@ -196,12 +209,25 @@ void set_default_env(const char *s)
>>
>> if (himport_r(&env_htab, (char *)default_environment,
>> sizeof(default_environment), '\0', 0,
>> - 0, NULL, 0 /* do_apply */) == 0)
>> + 0, NULL, do_apply) == 0)
>> error("Environment import failed: errno = %d\n", errno);
>>
>> gd->flags |= GD_FLG_ENV_READY;
>> }
>>
>> +
>> +/* [re]set individual variables to their value in the default environment
>> */ +int set_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, 1 /* do_apply */);
>> +}
>> +
>> /*
>> * Check if CRC is valid and (if yes) import the environment.
>> * Note that "buf" may or may not be aligned.
>> diff --git a/include/environment.h b/include/environment.h
>> index 90fb130..e8ab703 100644
>> --- a/include/environment.h
>> +++ b/include/environment.h
>> @@ -181,6 +181,9 @@ void env_crc_update(void);
>> /* [re]set to the default environment */
>> void set_default_env(const char *s);
>>
>> +/* [re]set individual variables to their value in the default environment
>> */ +int set_default_vars(int nvars, char * const vars[]);
>> +
>> /* Import from binary representation into hash table */
>> int env_import(const char *buf, int check);
>
> Best regards,
> Marek Vasut

Best regards,
Gerlando