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

[Marek Vasut]

Dear Gerlando Falauto,

> 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 at 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).

That doesn't mean it's right, it just means the codebase is in a very sad state 
:-C

> > Don't we have some getopt or something too ?
> 
> Not that I (or "git grep") know of.

Even more :-C

> >> +		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