| Message ID | 1345803102-21110-7-git-send-email-gerlando.falauto@keymile.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
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
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@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). > Don't we have some getopt or something too ? Not that I (or "git grep") know of. > >> + 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
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@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
On 08/24/2012 11:10 PM, Marek Vasut wrote: > 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@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 > You're absolutely right, but this patchset has been around for almost a year and we've gotten nowhere with it. I'm afraid that if we keep adding new (somewhat unrelated) improvement requests we'll get stuck with it for quite some time once again. Could we keep this getopt topic in mind as an open point for a later, overall review of how command line options are parsed? Thank you, Gerlando
Dear Gerlando Falauto, > On 08/24/2012 11:10 PM, Marek Vasut wrote: > > 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@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 > > You're absolutely right, but this patchset has been around for almost a > year and we've gotten nowhere with it. I'm afraid that if we keep adding > new (somewhat unrelated) improvement requests we'll get stuck with it > for quite some time once again. Definitelly, it was just a remark that it's sad state of the codebase, not that you should start implementing it ! > Could we keep this getopt topic in mind as an open point for a later, > overall review of how command line options are parsed? Yes! > Thank you, > Gerlando Best regards, Marek Vasut
Dear Marek Vasut, In message <201208242310.05189.marek.vasut@gmail.com> you wrote: > > > >> + 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 Define "right". And what makes you think the way K&R implemented such stuff would be a "very sad state"? See for example the code from Unix Version 6: "ls" command: if (--argc > 0 && *argv[1] == '-') { or "grep": while (--argc > 0 && (++argv)[0][0]=='-') etc. etc. Any C beginner's class is supposed to be able to parse this... Best regards, Wolfgang Denk
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 == '-') { + 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);
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(-)