Re: [U-Boot] [PATCH] env: Provide programmatic equivalent to 'setenv -f'

Simon Goldschmidt Tue, 19 Nov 2019 12:31:17 -0800

Am 19.11.2019 um 18:31 schrieb James Byrne:

Add env_force() to provide an equivalent to 'setenv -f' that can be used
programmatically.


Also tighten up the definition of argv in _do_env_set() so that
'const char *' pointers are used.

Signed-off-by: James Byrne <james.by...@origamienergy.com>


OK, I'm on CC, so I'll give my two cent:

I always thought this code to be messed up a bit: I think it's betterprogramming style to have the "string argument parsing" code call real Cfunctions with typed arguments. The env code instead does it the otherway round (here, you add do_programmatic_env_set() that sets up anargv[] array to call another function).

I'm not a maintainer for the ENV code, but maybe that should be sortedout instead of adding yet more code that goes this way?


Regards,
Simon


---

  cmd/nvedit.c  | 43 +++++++++++++++++++++++++++++--------------
  include/env.h | 13 +++++++++++++
  2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 99a3bc57b1..1f363ba9f4 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -221,10 +221,12 @@ DONE:
   * Set a new environment variable,
   * or replace or delete an existing one.
   */
-static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
+static int _do_env_set(int flag, int argc, const char * const argv[],
+                      int env_flag)
  {
        int   i, len;
-       char  *name, *value, *s;
+       const char *name;
+       char *value, *s;
        struct env_entry e, *ep;

debug("Initial value for argc=%d\n", argc);

@@ -235,7 +237,7 @@ static int _do_env_set(int flag, int argc, char * const 
argv[], int env_flag)
  #endif

while (argc > 1 && **(argv + 1) == '-') {

-               char *arg = *++argv;
+               const char *arg = *++argv;

--argc;

                while (*++arg) {
@@ -277,7 +279,7 @@ static int _do_env_set(int flag, int argc, char * const 
argv[], int env_flag)
                return 1;
        }
        for (i = 2, s = value; i < argc; ++i) {
-               char *v = argv[i];
+               const char *v = argv[i];

while ((*s++ = *v++) != '\0')

                        ;
@@ -299,18 +301,30 @@ static int _do_env_set(int flag, int argc, char * const 
argv[], int env_flag)
        return 0;
  }

-int env_set(const char *varname, const char *varvalue)

+static int do_programmatic_env_set(int argc, const char * const argv[])
  {
-       const char * const argv[4] = { "setenv", varname, varvalue, NULL };
-
        /* before import into hashtable */
        if (!(gd->flags & GD_FLG_ENV_READY))
                return 1;

- if (varvalue == NULL || varvalue[0] == '\0')

-               return _do_env_set(0, 2, (char * const *)argv, H_PROGRAMMATIC);
-       else
-               return _do_env_set(0, 3, (char * const *)argv, H_PROGRAMMATIC);
+       if (!argv[argc - 1] || argv[argc - 1][0] == '\0')
+               argc--;
+
+       return _do_env_set(0, argc, argv, H_PROGRAMMATIC);
+}
+
+int env_set(const char *varname, const char *varvalue)
+{
+       const char * const argv[4] = {"setenv", varname, varvalue, NULL};
+
+       return do_programmatic_env_set(3, argv);
+}
+
+int env_force(const char *varname, const char *varvalue)
+{
+       const char * const argv[5] = {"setenv", "-f", varname, varvalue, NULL};
+
+       return do_programmatic_env_set(4, argv);
  }

/**

@@ -382,7 +396,8 @@ static int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
        if (argc < 2)
                return CMD_RET_USAGE;

- return _do_env_set(flag, argc, argv, H_INTERACTIVE);

+       return _do_env_set(flag, argc, (const char * const *)argv,
+                          H_INTERACTIVE);
  }

/*

@@ -643,12 +658,12 @@ static int do_env_edit(cmd_tbl_t *cmdtp, int flag, int 
argc,
        if (buffer[0] == '\0') {
                const char * const _argv[3] = { "setenv", argv[1], NULL };

- return _do_env_set(0, 2, (char * const *)_argv, H_INTERACTIVE);

+               return _do_env_set(0, 2, _argv, H_INTERACTIVE);
        } else {
                const char * const _argv[4] = { "setenv", argv[1], buffer,
                        NULL };

- return _do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE);

+               return _do_env_set(0, 3, _argv, H_INTERACTIVE);
        }
  }
  #endif /* CONFIG_CMD_EDITENV */
diff --git a/include/env.h b/include/env.h
index b72239f6a5..37bbf1117d 100644
--- a/include/env.h
+++ b/include/env.h
@@ -145,6 +145,19 @@ int env_get_yesno(const char *var);
   */
  int env_set(const char *varname, const char *value);

+/**

+ * env_force() - forcibly set an environment variable
+ *
+ * This sets or deletes the value of an environment variable. It is the same
+ * as env_set(), except that the variable will be forcibly updated/deleted,
+ * even if it has access protection flags set.
+ *
+ * @varname: Variable to adjust
+ * @value: Value to set for the variable, or NULL or "" to delete the variable
+ * @return 0 if OK, 1 on error
+ */
+int env_force(const char *varname, const char *varvalue);
+
  /**
   * env_get_ulong() - Return an environment variable as an integer value
   *


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] env: Provide programmatic equivalent to 'setenv -f'

Reply via email to