Dear Gerlando Falauto,

> On 03/29/2012 10:25 PM, Marek Vasut wrote:
> > 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.fala...@keymile.com>
> > 
> > I have to admit I'm not much of a fan of how you use this apply()
> > callback, is it really necessary?
> 
> See my previous email.
> 
> > Also, do we need special command for default env?
> 
> I am sorry, I don't get the question. What do you mean?
> It's always the same "env import" with an augmented command line.
> If you're referring to CONFIG_CMD_DEFAULTENV_VARS, I don't think it's
> neeed either, but it was requested at some previous time.

Hmm.

> I also think it's misleading to have some syntax for the same command
> conditionally enabled. Has it ever been done before?

Yes it was, but this is creepy -- is this command displaying the compiled-in 
set 
of env vars?

> 
> Thank you,
> Gerlando
> 
> > 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);
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to