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

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. Don't we have some getopt or something too ?

• 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