Skip to contentSkip to conversation

env: add command to set individual variables to default

RFC PATCH New Gerlando Falauto 3 participants 3 messages Sep 8, 2011 – Oct 6, 2011

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 env: add command to set individual variables to default

Here I am proposing an implementation for setting individual variables to their default values as outlined in
http://www.denx.de/wiki/U-Boot/TaskSetEnvironmentDefaults

For instance, to reset defautl values for variables bootcmd and bootdelay:
=> env default bootcmd bootdelay

There are a few issues which are not fully covered. [In braces I put the behavior of this patch].

1) Previous implementation of ?env default? only worked for the whole environment, and ?-f? was necessary in order to prevent the unexperienced user from messing up the environment by just typing command names. Should this behavior be kept? [Yes]

2) When a variable is not defined in the default environment, should it get deleted (if existing)? [Yes] Should a warning be issued? [No] What if it is neither defined in the current nor in the default env?

3) Should it be possible to disable this feature (i.e. to save space)? [No]
Any suggestions for a meaningful configuration token?
I was thinking of CONFIG_RESTORE_INDIVIDUAL_ENV_VARS.

4) This implementation comes mostly from himport_r(). It crossed my mind that it might also be useful to *import* individual variables from a file (rather that the default env).

For instance:
env import [flags] addr size vars...

Would only import variables "vars" and ignore all others.

With this idea in mind I kept all himport_r()'s support for external files (i.e., comments, separator other than '\0', quoting).
If no one else envisions any use for such import commands, the code could obviously be much smaller and simpler.

---
 common/cmd_nvedit.c   |   12 +++++-
 common/env_common.c   |  106 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |    3 +
 3 files changed, 119 insertions(+), 2 deletions(-)
common/cmd_nvedit.c +10 -2
index b2c88ba..9822785 100644
@@ -580,10 +580,17 @@ int envmatch(uchar *s1, int i2)
 
 static int do_env_default(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	if ((argc != 2) || (strcmp(argv[1], "-f") != 0))
+	/* Check that we have at least one argument */
+	if (argc < 2) {
 		return cmd_usage(cmdtp);
+	} else if ((argc == 2) && (strcmp(argv[1], "-f")) == 0) {
+		/* Reset the whole environment */
+		set_default_env("## Resetting to default environment\n");
+		return 0;
+	}
 
-	set_default_env("## Resetting to default environment\n");
+	/* Reset individual variables */
+	env_default_vars(argc-1, argv+1);
 	return 0;
 }
 
@@ -910,6 +917,7 @@ U_BOOT_CMD(
 	"ask name [message] [size] - ask for environment variable\nenv "
 #endif
 	"default -f - reset default environment\n"
+	"env default name [...] - reset variable(s) to their default value\n"
 #if defined(CONFIG_CMD_EDITENV)
 	"env edit name - edit environment variable\n"
 #endif
common/env_common.c +106 -0

112 lines not on this page · raw message

include/environment.h +3 -0
index 53d92df..2c4d905 100644
@@ -169,6 +169,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 env_default_vars(int nvars, char * const vars[]);
+
 /* Import from binary representation into hash table */
 int env_import(const char *buf, int check);
 
Signature · 1 line
1.7.1
#1 lore raw

RFC PATCH 1/1 env: add command to set individual variables to default

Quoted 6 lines · On Thursday, September 08, 2011 04:49:28 Gerlando Falauto wrote:

Here I am proposing an implementation for setting individual variables to their default values as outlined in
http://www.denx.de/wiki/U-Boot/TaskSetEnvironmentDefaults

For instance, to reset defautl values for variables bootcmd and bootdelay:
=> env default bootcmd bootdelay

please put this behind a CONFIG so it can be turned off. i have no preference for the default (defining it in config_defaults.h).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110908/4fd50abf/attachment.pgp

#2 lore raw

RFC PATCH 1/1 env: add command to set individual variables to default

In reply to Gerlando Falauto

Dear Gerlando Falauto,

Quoted 38 lines · In message <[email protected]>…

Here I am proposing an implementation for setting individual variables to their default values as outlined in
http://www.denx.de/wiki/U-Boot/TaskSetEnvironmentDefaults

For instance, to reset defautl values for variables bootcmd and bootdelay:
=> env default bootcmd bootdelay

There are a few issues which are not fully covered. [In braces I put the behavior of this patch].

1) Previous implementation of ???env default??? only worked for the whole environment, and ???-f??? was necessary in order to prevent the unexperienced user from messing up the environment by just typing command names. Should this behavior be kept? [Yes]

2) When a variable is not defined in the default environment, should it get deleted (if existing)? [Yes] Should a warning be issued? [No] What if it is neither defined in the current nor in the default env?

3) Should it be possible to disable this feature (i.e. to save space)? [No]
Any suggestions for a meaningful configuration token?
I was thinking of CONFIG_RESTORE_INDIVIDUAL_ENV_VARS.

4) This implementation comes mostly from himport_r(). It crossed my mind that it might also be useful to *import* individual variables from a file (rather that the default env).

For instance:
env import [flags] addr size vars...

Would only import variables "vars" and ignore all others.

With this idea in mind I kept all himport_r()'s support for external files (i.e., comments, separator other than '\0', quoting).
If no one else envisions any use for such import commands, the code could obviously be much smaller and simpler.

Quoted 8 lines
---
 common/cmd_nvedit.c   |   12 +++++-
 common/env_common.c   |  106 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/environment.h |    3 +
 3 files changed, 119 insertions(+), 2 deletions(-)

Checkpatch says:

total: 5 errors, 0 warnings, 147 lines checked

Please clean up and resubmit. Thanks.

Best regards,

Wolfgang Denk

Signature · 12 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Zero is an enigmatic value. It can mean success (fclose)  or  failure
(scanf).  It  can  mean  black  or  white. It can mean no permissions
(chmod) or all permissions (umask). It can mean now (setjmp) or later
(atexit). It can mean the beginning (lseek) or the end (read). It can
mean myself (getpgrp) or child (fork). It can mean  all  (kill's  1st
argument)  or  nothing  (kill's  2nd argument). It can mean `default'
(SIG_IGN) or `I don't care' (waitpid) or  `try  to  guess'  (strtol).
Indeed 0 lets you talk to God (setuid).
Verily is 0 all things to all people.