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

[Marek Vasut]

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 at keymile.com>

I have to admit I'm not much of a fan of how you use this apply() callback, is 
it really necessary?

Also, do we need special command for default env?

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);