env: Refactor do_apply to a flag

Use a flag in hsearch_r for insert mode passed from import to allow the
behavior be different based on use.

Now that "do_check" is called for all imports, ensure console init is
complete before updating the console on relocation import

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

common/cmd_nvedit.c

@@ -116,7 +116,7 @@ static int env_print(char *name)
 
 		e.key = name;
 		e.data = NULL;
-		hsearch_r(e, FIND, &ep, &env_htab);
+		hsearch_r(e, FIND, &ep, &env_htab, 0);
 		if (ep == NULL)
 			return 0;
 		len = printf("%s=%s\n", ep->key, ep->data);
@@ -224,7 +224,7 @@ int env_check_apply(const char *name, const char *oldval,
 	else if (strcmp(name, "stderr") == 0)
 		console = stderr;
 
-	if (console != -1) {
+	if (console != -1 && (gd->flags & GD_FLG_DEVINIT) != 0) {
 		if ((newval == NULL) || (*newval == '\0')) {
 			/* We cannot delete stdin/stdout/stderr */
 			if ((flag & H_FORCE) == 0)
@@ -344,20 +344,19 @@ static int _do_env_set(int flag, int argc, char * const argv[])
 	 */
 	e.key = name;
 	e.data = NULL;
-	hsearch_r(e, FIND, &ep, &env_htab);
+	hsearch_r(e, FIND, &ep, &env_htab, 0);
 
 	/*
-	 * Perform requested checks. Notice how since we are overwriting
-	 * a single variable, we need to set H_NOCLEAR
+	 * Perform requested checks.
 	 */
-	if (env_check_apply(name, ep ? ep->data : NULL, value, H_NOCLEAR)) {
+	if (env_check_apply(name, ep ? ep->data : NULL, value, 0)) {
 		debug("check function did not approve, refusing\n");
 		return 1;
 	}
 
 	/* Delete only ? */
 	if (argc < 3 || argv[2] == NULL) {
-		int rc = hdelete_r(name, &env_htab, 0);
+		int rc = hdelete_r(name, &env_htab, H_INTERACTIVE);
 		return !rc;
 	}
 
@@ -384,7 +383,7 @@ static int _do_env_set(int flag, int argc, char * const argv[])
 
 	e.key	= name;
 	e.data	= value;
-	hsearch_r(e, ENTER, &ep, &env_htab);
+	hsearch_r(e, ENTER, &ep, &env_htab, H_INTERACTIVE);
 	free(value);
 	if (!ep) {
 		printf("## Error inserting \"%s\" variable, errno=%d\n",
@@ -552,7 +551,7 @@ char *getenv(const char *name)
 
 		e.key	= name;
 		e.data	= NULL;
-		hsearch_r(e, FIND, &ep, &env_htab);
+		hsearch_r(e, FIND, &ep, &env_htab, 0);
 
 		return ep ? ep->data : NULL;
 	}
@@ -951,7 +950,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 	}
 
 	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
-			0, NULL, 0 /* do_apply */) == 0) {
+			0, NULL) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 		return 1;
 	}

common/console.c

@@ -681,8 +681,6 @@ int console_init_r(void)
 done:
 #endif
 
-	gd->flags |= GD_FLG_DEVINIT;	/* device initialization completed */
-
 #ifndef CONFIG_SYS_CONSOLE_INFO_QUIET
 	stdio_print_current_devices();
 #endif /* CONFIG_SYS_CONSOLE_INFO_QUIET */
@@ -694,6 +692,8 @@ done:
 	}
 #endif /* CONFIG_SYS_CONSOLE_ENV_OVERWRITE */
 
+	gd->flags |= GD_FLG_DEVINIT;	/* device initialization completed */
+
 #if 0
 	/* If nothing usable installed, use only the initial console */
 	if ((stdio_devices[stdin] == NULL) && (stdio_devices[stdout] == NULL))
@@ -758,8 +758,6 @@ int console_init_r(void)
 #endif
 	}
 
-	gd->flags |= GD_FLG_DEVINIT;	/* device initialization completed */
-
 #ifndef CONFIG_SYS_CONSOLE_INFO_QUIET
 	stdio_print_current_devices();
 #endif /* CONFIG_SYS_CONSOLE_INFO_QUIET */
@@ -769,6 +767,8 @@ int console_init_r(void)
 		setenv(stdio_names[i], stdio_devices[i]->name);
 	}
 
+	gd->flags |= GD_FLG_DEVINIT;	/* device initialization completed */
+
 #if 0
 	/* If nothing usable installed, use only the initial console */
 	if ((stdio_devices[stdin] == NULL) && (stdio_devices[stdout] == NULL))

common/env_common.c

@@ -83,11 +83,8 @@ 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;
+	int flags = 0;
+
 	if (sizeof(default_environment) > ENV_SIZE) {
 		puts("*** Error - default environment is too large\n\n");
 		return;
@@ -99,14 +96,7 @@ 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;
+			flags = H_INTERACTIVE;
 			puts(s);
 		}
 	} else {
@@ -114,8 +104,8 @@ void set_default_env(const char *s)
 	}
 
 	if (himport_r(&env_htab, (char *)default_environment,
-			sizeof(default_environment), '\0', 0,
-			0, NULL, do_apply) == 0)
+			sizeof(default_environment), '\0', flags,
+			0, NULL) == 0)
 		error("Environment import failed: errno = %d\n", errno);
 
 	gd->flags |= GD_FLG_ENV_READY;
@@ -130,8 +120,8 @@ int set_default_vars(int nvars, char * const vars[])
 	 * (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 */);
+				sizeof(default_environment), '\0',
+				H_NOCLEAR | H_INTERACTIVE, nvars, vars);
 }
 
 #ifndef CONFIG_SPL_BUILD
@@ -155,7 +145,7 @@ int env_import(const char *buf, int check)
 	}
 
 	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
-			0, NULL, 0 /* do_apply */)) {
+			0, NULL)) {
 		gd->flags |= GD_FLG_ENV_READY;
 		return 1;
 	}

include/search.h

@@ -73,7 +73,7 @@ struct hsearch_data {
 extern int hcreate_r(size_t __nel, struct hsearch_data *__htab);
 
 /* Destroy current internal hashing table.  */
-extern void hdestroy_r(struct hsearch_data *__htab, int do_apply);
+extern void hdestroy_r(struct hsearch_data *__htab);
 
 /*
  * Search for entry matching ITEM.key in internal hash table.  If
@@ -82,7 +82,7 @@ extern void hdestroy_r(struct hsearch_data *__htab, int do_apply);
  * ITEM.data.
  * */
 extern int hsearch_r(ENTRY __item, ACTION __action, ENTRY ** __retval,
-		     struct hsearch_data *__htab);
+		     struct hsearch_data *__htab, int __flag);
 
 /*
  * Search for an entry matching `MATCH'.  Otherwise, Same semantics
@@ -99,7 +99,7 @@ extern int hstrstr_r(const char *__match, int __last_idx, ENTRY ** __retval,
 
 /* Search and delete entry matching ITEM.key in internal hash table. */
 extern int hdelete_r(const char *__key, struct hsearch_data *__htab,
-			int do_apply);
+		     int __flag);
 
 extern ssize_t hexport_r(struct hsearch_data *__htab,
 		     const char __sep, char **__resp, size_t __size,
@@ -113,10 +113,11 @@ extern ssize_t hexport_r(struct hsearch_data *__htab,
  */
 extern int himport_r(struct hsearch_data *__htab,
 		     const char *__env, size_t __size, const char __sep,
-		     int __flag, int nvars, char * const vars[], int do_apply);
+		     int __flag, int nvars, char * const vars[]);
 
-/* Flags for himport_r() */
-#define	H_NOCLEAR	(1 << 0) /* do not clear hash table before importing */
-#define	H_FORCE		(1 << 1) /* overwrite read-only/write-once variables */
+/* Flags for himport_r(), hdelete_r(), and hsearch_r() */
+#define H_NOCLEAR	(1 << 0) /* do not clear hash table before importing */
+#define H_FORCE		(1 << 1) /* overwrite read-only/write-once variables */
+#define H_INTERACTIVE	(1 << 2) /* indicate that an import is user directed */
 
 #endif /* search.h */

lib/hashtable.c

@@ -142,7 +142,7 @@ int hcreate_r(size_t nel, struct hsearch_data *htab)
  * be freed and the local static variable can be marked as not used.
  */
 
-void hdestroy_r(struct hsearch_data *htab, int do_apply)
+void hdestroy_r(struct hsearch_data *htab)
 {
 	int i;
 
@@ -156,10 +156,7 @@ void hdestroy_r(struct hsearch_data *htab, int do_apply)
 	for (i = 1; i <= htab->size; ++i) {
 		if (htab->table[i].used > 0) {
 			ENTRY *ep = &htab->table[i].entry;
-			if (do_apply && htab->apply != NULL) {
-				/* deletion is always forced */
-				htab->apply(ep->key, ep->data, NULL, H_FORCE);
-			}
+
 			free((void *)ep->key);
 			free(ep->data);
 		}
@@ -251,7 +248,7 @@ int hmatch_r(const char *match, int last_idx, ENTRY ** retval,
 }
 
 int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
-	      struct hsearch_data *htab)
+	      struct hsearch_data *htab, int flag)
 {
 	unsigned int hval;
 	unsigned int count;
@@ -404,7 +401,7 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
  * do that.
  */
 
-int hdelete_r(const char *key, struct hsearch_data *htab, int do_apply)
+int hdelete_r(const char *key, struct hsearch_data *htab, int flag)
 {
 	ENTRY e, *ep;
 	int idx;
@@ -413,15 +410,21 @@ int hdelete_r(const char *key, struct hsearch_data *htab, int do_apply)
 
 	e.key = (char *)key;
 
-	if ((idx = hsearch_r(e, FIND, &ep, htab)) == 0) {
+	idx = hsearch_r(e, FIND, &ep, htab, 0);
+	if (idx == 0) {
 		__set_errno(ESRCH);
 		return 0;	/* not found */
 	}
 
+	/* Check for permission */
+	if (htab->apply != NULL &&
+	    htab->apply(ep->key, ep->data, NULL, flag)) {
+		__set_errno(EPERM);
+		return 0;
+	}
+
 	/* free used ENTRY */
 	debug("hdelete: DELETING key \"%s\"\n", key);
-	if (do_apply && htab->apply != NULL)
-		htab->apply(ep->key, ep->data, NULL, H_FORCE);
 	free((void *)ep->key);
 	free(ep->data);
 	htab->table[idx].used = -1;
@@ -674,7 +677,7 @@ static int drop_var_from_set(const char *name, int nvars, char * vars[])
 
 int himport_r(struct hsearch_data *htab,
 		const char *env, size_t size, const char sep, int flag,
-		int nvars, char * const vars[], int do_apply)
+		int nvars, char * const vars[])
 {
 	char *data, *sp, *dp, *name, *value;
 	char *localvars[nvars];
@@ -704,7 +707,7 @@ int himport_r(struct hsearch_data *htab,
 		debug("Destroy Hash Table: %p table = %p\n", htab,
 		       htab->table);
 		if (htab->table)
-			hdestroy_r(htab, do_apply);
+			hdestroy_r(htab);
 	}
 
 	/*
@@ -770,7 +773,7 @@ int himport_r(struct hsearch_data *htab,
 			if (!drop_var_from_set(name, nvars, localvars))
 				continue;
 
-			if (hdelete_r(name, htab, do_apply) == 0)
+			if (hdelete_r(name, htab, flag) == 0)
 				debug("DELETE ERROR ##############################\n");
 
 			continue;
@@ -795,14 +798,14 @@ int himport_r(struct hsearch_data *htab,
 		e.data = value;
 
 		/* if there is an apply function, check what it has to say */
-		if (do_apply && htab->apply != NULL) {
+		if (htab->apply != NULL) {
 			debug("searching before calling cb function"
 				" for  %s\n", name);
 			/*
 			 * Search for variable in existing env, so to pass
 			 * its previous value to the apply callback
 			 */
-			hsearch_r(e, FIND, &rv, htab);
+			hsearch_r(e, FIND, &rv, htab, 0);
 			debug("previous value was %s\n", rv ? rv->data : "");
 			if (htab->apply(name, rv ? rv->data : NULL,
 				value, flag)) {
@@ -812,7 +815,7 @@ int himport_r(struct hsearch_data *htab,
 			}
 		}
 
-		hsearch_r(e, ENTER, &rv, htab);
+		hsearch_r(e, ENTER, &rv, htab, flag);
 		if (rv == NULL) {
 			printf("himport_r: can't insert \"%s=%s\" into hash table\n",
 				name, value);
@@ -839,7 +842,7 @@ int himport_r(struct hsearch_data *htab,
 		 * b) if the variable was not present in current env, we notify
 		 *    it might be a typo
 		 */
-		if (hdelete_r(localvars[i], htab, do_apply) == 0)
+		if (hdelete_r(localvars[i], htab, flag) == 0)
 			printf("WARNING: '%s' neither in running nor in imported env!\n", localvars[i]);
 		else
 			printf("WARNING: '%s' not in imported env, deleting it!\n", localvars[i]);