[U-Boot] [PATCH v2 3/3] env: make "env default" selective, check and apply

[Gerlando Falauto <gerlando.falauto@keymile.com>]

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<gerlando.falauto@keymile.com>
>
> 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);