env: Move env_set() out of cmd/nvedit.c and in to env/common.c

Inside of env/common.c we already have our helper env_set_xxx functions,
and even have a comment that explains why env_set() itself wasn't moved.
We now handle that move. This requires that we rename the previous
_do_env_set() to env_do_env_set() and note it as an internal env
function. Add comments about this function to explain why we do this
when we add the prototype. Add a new function, env_inc_id() to allow for
the counter to be updated by both commands and callers, and document
this as well by the prototype.

Signed-off-by: Tom Rini <trini@konsulko.com>

diff --git a/cmd/Makefile b/cmd/Makefile
index 9a6790cc170850931a6026017e7645a28537128f..dbeeebe2c4b1938df730a3223fdee515046c7790 100644 (file)
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -128,6 +128,7 @@ endif
 obj-$(CONFIG_CMD_MUX) += mux.o
 obj-$(CONFIG_CMD_NAND) += nand.o
 obj-$(CONFIG_CMD_NET) += net.o
+obj-$(CONFIG_ENV_SUPPORT) += nvedit.o
 obj-$(CONFIG_CMD_NVEDIT_EFI) += nvedit_efi.o
 obj-$(CONFIG_CMD_ONENAND) += onenand.o
 obj-$(CONFIG_CMD_OSD) += osd.o
@@ -245,9 +246,6 @@ endif # !CONFIG_SPL_BUILD
 
 obj-$(CONFIG_$(SPL_)CMD_TLV_EEPROM) += tlv_eeprom.o
 
-# core command
-obj-y += nvedit.o
-
 obj-$(CONFIG_CMD_BCM_EXT_UTILS) += broadcom/
 
 filechk_data_gz = (echo "static const char data_gz[] ="; cat $< | scripts/bin2c; echo ";")

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index daf1ad37f9be25477e63e058061395922c55b12b..e77338f81394b3f92fb23779bd7dfc30266e7f7f 100644 (file)
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -49,20 +49,6 @@ DECLARE_GLOBAL_DATA_PTR;
  */
 #define        MAX_ENV_SIZE    (1 << 20)       /* 1 MiB */
 
-/*
- * This variable is incremented on each do_env_set(), so it can
- * be used via env_get_id() as an indication, if the environment
- * has changed or not. So it is possible to reread an environment
- * variable only if the environment was changed ... done so for
- * example in NetInitLoop()
- */
-static int env_id = 1;
-
-int env_get_id(void)
-{
-       return env_id;
-}
-
 #ifndef CONFIG_SPL_BUILD
 /*
  * Command interface: print one or all environment variables
@@ -198,104 +184,6 @@ DONE:
 #endif
 #endif /* CONFIG_SPL_BUILD */
 
-/*
- * 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)
-{
-       int   i, len;
-       char  *name, *value, *s;
-       struct env_entry e, *ep;
-
-       debug("Initial value for argc=%d\n", argc);
-
-#if !IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_CMD_NVEDIT_EFI)
-       if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e')
-               return do_env_set_efi(NULL, flag, --argc, ++argv);
-#endif
-
-       while (argc > 1 && **(argv + 1) == '-') {
-               char *arg = *++argv;
-
-               --argc;
-               while (*++arg) {
-                       switch (*arg) {
-                       case 'f':               /* force */
-                               env_flag |= H_FORCE;
-                               break;
-                       default:
-                               return CMD_RET_USAGE;
-                       }
-               }
-       }
-       debug("Final value for argc=%d\n", argc);
-       name = argv[1];
-
-       if (strchr(name, '=')) {
-               printf("## Error: illegal character '='"
-                      "in variable name \"%s\"\n", name);
-               return 1;
-       }
-
-       env_id++;
-
-       /* Delete only ? */
-       if (argc < 3 || argv[2] == NULL) {
-               int rc = hdelete_r(name, &env_htab, env_flag);
-
-               /* If the variable didn't exist, don't report an error */
-               return rc && rc != -ENOENT ? 1 : 0;
-       }
-
-       /*
-        * Insert / replace new value
-        */
-       for (i = 2, len = 0; i < argc; ++i)
-               len += strlen(argv[i]) + 1;
-
-       value = malloc(len);
-       if (value == NULL) {
-               printf("## Can't malloc %d bytes\n", len);
-               return 1;
-       }
-       for (i = 2, s = value; i < argc; ++i) {
-               char *v = argv[i];
-
-               while ((*s++ = *v++) != '\0')
-                       ;
-               *(s - 1) = ' ';
-       }
-       if (s != value)
-               *--s = '\0';
-
-       e.key   = name;
-       e.data  = value;
-       hsearch_r(e, ENV_ENTER, &ep, &env_htab, env_flag);
-       free(value);
-       if (!ep) {
-               printf("## Error inserting \"%s\" variable, errno=%d\n",
-                       name, errno);
-               return 1;
-       }
-
-       return 0;
-}
-
-int env_set(const char *varname, const char *varvalue)
-{
-       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);
-}
-
 #ifndef CONFIG_SPL_BUILD
 static int do_env_set(struct cmd_tbl *cmdtp, int flag, int argc,
                      char *const argv[])
@@ -303,7 +191,7 @@ static int do_env_set(struct cmd_tbl *cmdtp, int flag, int argc,
        if (argc < 2)
                return CMD_RET_USAGE;
 
-       return _do_env_set(flag, argc, argv, H_INTERACTIVE);
+       return env_do_env_set(flag, argc, argv, H_INTERACTIVE);
 }
 
 /*
@@ -381,7 +269,7 @@ int do_env_ask(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
        }
 
        /* Continue calling setenv code */
-       return _do_env_set(flag, len, local_args, H_INTERACTIVE);
+       return env_do_env_set(flag, len, local_args, H_INTERACTIVE);
 }
 #endif
 
@@ -561,12 +449,12 @@ static int do_env_edit(struct cmd_tbl *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 env_do_env_set(0, 2, (char * const *)_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 env_do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE);
        }
 }
 #endif /* CONFIG_CMD_EDITENV */
@@ -679,7 +567,7 @@ static int do_env_delete(struct cmd_tbl *cmdtp, int flag,
        }
        debug("Final value for argc=%d\n", argc);
 
-       env_id++;
+       env_inc_id();
 
        while (--argc > 0) {
                char *name = *++argv;

diff --git a/env/common.c b/env/common.c
index eb1a91379539b500676058628a9b21967cfd3ccf..656748c1f5b72cf9ddfec5ea3611b85fab207c41 100644 (file)
--- a/env/common.c
+++ b/env/common.c
@@ -37,11 +37,116 @@ struct hsearch_data env_htab = {
 };
 
 /*
- * This env_set() function is defined in cmd/nvedit.c, since it calls
- * _do_env_set(), whis is a static function in that file.
- *
- * int env_set(const char *varname, const char *varvalue);
+ * This variable is incremented each time we set an environment variable so we
+ * can be check via env_get_id() to see if the environment has changed or not.
+ * This makes it possible to reread an environment variable only if the
+ * environment was changed, typically used by networking code.
  */
+static int env_id = 1;
+
+int env_get_id(void)
+{
+       return env_id;
+}
+
+void env_inc_id(void)
+{
+       env_id++;
+}
+
+int env_do_env_set(int flag, int argc, char *const argv[], int env_flag)
+{
+       int   i, len;
+       char  *name, *value, *s;
+       struct env_entry e, *ep;
+
+       debug("Initial value for argc=%d\n", argc);
+
+#if !IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_CMD_NVEDIT_EFI)
+       if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e')
+               return do_env_set_efi(NULL, flag, --argc, ++argv);
+#endif
+
+       while (argc > 1 && **(argv + 1) == '-') {
+               char *arg = *++argv;
+
+               --argc;
+               while (*++arg) {
+                       switch (*arg) {
+                       case 'f':               /* force */
+                               env_flag |= H_FORCE;
+                               break;
+                       default:
+                               return CMD_RET_USAGE;
+                       }
+               }
+       }
+       debug("Final value for argc=%d\n", argc);
+       name = argv[1];
+
+       if (strchr(name, '=')) {
+               printf("## Error: illegal character '='"
+                      "in variable name \"%s\"\n", name);
+               return 1;
+       }
+
+       env_inc_id();
+
+       /* Delete only ? */
+       if (argc < 3 || argv[2] == NULL) {
+               int rc = hdelete_r(name, &env_htab, env_flag);
+
+               /* If the variable didn't exist, don't report an error */
+               return rc && rc != -ENOENT ? 1 : 0;
+       }
+
+       /*
+        * Insert / replace new value
+        */
+       for (i = 2, len = 0; i < argc; ++i)
+               len += strlen(argv[i]) + 1;
+
+       value = malloc(len);
+       if (value == NULL) {
+               printf("## Can't malloc %d bytes\n", len);
+               return 1;
+       }
+       for (i = 2, s = value; i < argc; ++i) {
+               char *v = argv[i];
+
+               while ((*s++ = *v++) != '\0')
+                       ;
+               *(s - 1) = ' ';
+       }
+       if (s != value)
+               *--s = '\0';
+
+       e.key   = name;
+       e.data  = value;
+       hsearch_r(e, ENV_ENTER, &ep, &env_htab, env_flag);
+       free(value);
+       if (!ep) {
+               printf("## Error inserting \"%s\" variable, errno=%d\n",
+                       name, errno);
+               return 1;
+       }
+
+       return 0;
+}
+
+int env_set(const char *varname, const char *varvalue)
+{
+       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 env_do_env_set(0, 2, (char * const *)argv, H_PROGRAMMATIC);
+       else
+               return env_do_env_set(0, 3, (char * const *)argv, H_PROGRAMMATIC);
+}
 
 /**
  * Set an environment variable to an integer value

diff --git a/include/env.h b/include/env.h
index 430c4fa94a426362e8060f3730ef2fb6f0135d71..9778e3e4f2ce33e3530e3a941357a8df92c739bf 100644 (file)
--- a/include/env.h
+++ b/include/env.h
@@ -72,6 +72,14 @@ enum env_redund_flags {
  */
 int env_get_id(void);
 
+/**
+ * env_inc_id() - Increase the sequence number for the environment
+ *
+ * Increment the value that is used by env_get_id() to inform callers
+ * if the environment has changed since they last checked.
+ */
+void env_inc_id(void);
+
 /**
  * env_init() - Set up the pre-relocation environment
  *

diff --git a/include/env_internal.h b/include/env_internal.h
index 6a69494646893aaeb227ebfb43839cd0ab76b20c..ae7816d38e588e802453b173aabe9d54720e8d32 100644 (file)
--- a/include/env_internal.h
+++ b/include/env_internal.h
@@ -193,6 +193,18 @@ struct env_driver {
 
 extern struct hsearch_data env_htab;
 
+/**
+ * env_do_env_set() - Perform the actual setting of an environment variable
+ *
+ * Due to the number of places we may need to set an environmental variable
+ * from we have an exposed internal function that performs the real work and
+ * then call this from both the command line function as well as other
+ * locations.
+ *
+ * Return: 0 on success or 1 on failure
+ */
+int env_do_env_set(int flag, int argc, char *const argv[], int env_flag);
+
 /**
  * env_ext4_get_intf() - Provide the interface for env in EXT4
  *