[U-Boot] [PATCH 09/12] env: Clarify the cases for env set

Joe Hershberger joe.hershberger at ni.com
Fri Aug 17 22:49:43 CEST 2012


Use variables that define the access type to the env.  This will
simplify validation of access with the ACL.

If deleting a variable that doesn't exist, do nothing

Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
---
 common/cmd_nvedit.c | 101 +++++++++++++++++++++++++++-------------------------
 tools/env/fw_env.c  |  69 +++++++++++++++++------------------
 2 files changed, 88 insertions(+), 82 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 76cdd87..0d5b957 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -207,6 +207,7 @@ int _do_env_set(int flag, int argc, char * const argv[])
 	int   console = -1;
 	char  *name, *value, *s;
 	ENTRY e, *ep;
+	int deleting, creating, overwriting;
 
 	name = argv[1];
 
@@ -224,6 +225,10 @@ int _do_env_set(int flag, int argc, char * const argv[])
 	e.data = NULL;
 	hsearch_r(e, FIND, &ep, &env_htab);
 
+	deleting = (ep && ((argc < 3) || argv[2] == NULL));
+	creating = (!ep && ((argc >= 3) && argv[2] != NULL));
+	overwriting = (ep && ((argc >= 3) && argv[2] != NULL));
+
 	/* Check for console redirection */
 	if (strcmp(name, "stdin") == 0)
 		console = stdin;
@@ -233,7 +238,7 @@ int _do_env_set(int flag, int argc, char * const argv[])
 		console = stderr;
 
 	if (console != -1) {
-		if (argc < 3) {		/* Cannot delete it! */
+		if (deleting) {		/* Cannot delete it! */
 			printf("Can't delete \"%s\"\n", name);
 			return 1;
 		}
@@ -258,7 +263,7 @@ int _do_env_set(int flag, int argc, char * const argv[])
 	 * Some variables like "ethaddr" and "serial#" can be set only
 	 * once and cannot be deleted; also, "ver" is readonly.
 	 */
-	if (ep) {		/* variable exists */
+	if (deleting || overwriting) {		/* variable exists */
 #ifndef CONFIG_ENV_OVERWRITE
 		if (strcmp(name, "serial#") == 0 ||
 		    (strcmp(name, "ethaddr") == 0
@@ -270,6 +275,9 @@ int _do_env_set(int flag, int argc, char * const argv[])
 			return 1;
 		}
 #endif
+	}
+
+	if (creating || overwriting) {
 		/*
 		 * Switch to new baudrate if new baudrate is supported
 		 */
@@ -300,8 +308,7 @@ int _do_env_set(int flag, int argc, char * const argv[])
 		}
 	}
 
-	/* Delete only ? */
-	if (argc < 3 || argv[2] == NULL) {
+	if (deleting) {
 		int rc = hdelete_r(name, &env_htab);
 
 #if defined(CONFIG_SILENT_CONSOLE) && \
@@ -314,59 +321,57 @@ int _do_env_set(int flag, int argc, char * const argv[])
 		return !rc;
 	}
 
+	if (creating || overwriting) {
 #if defined(CONFIG_SILENT_CONSOLE) && \
 	defined(CONFIG_SILENT_CONSOLE_UPDATE_ON_SET)
-	if (strcmp(name, "silent") == 0) {
-		/* silent is added */
-		gd->flags |= GD_FLG_SILENT;
-	}
+		if (strcmp(name, "silent") == 0)
+			/* silent is added */
+			gd->flags |= GD_FLG_SILENT;
 #endif
 
-	/*
-	 * Insert / replace new value
-	 */
-	for (i = 2, len = 0; i < argc; ++i)
-		len += strlen(argv[i]) + 1;
+		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];
+		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, ENTER, &ep, &env_htab);
-	free(value);
-	if (!ep) {
-		printf("## Error inserting \"%s\" variable, errno=%d\n",
-			name, errno);
-		return 1;
-	}
+			while ((*s++ = *v++) != '\0')
+				continue;
+			*(s - 1) = ' ';
+		}
+		if (s != value)
+			*--s = '\0';
 
-	/*
-	 * Some variables should be updated when the corresponding
-	 * entry in the environment is changed
-	 */
-	if (strcmp(argv[1], "loadaddr") == 0) {
-		load_addr = simple_strtoul(argv[2], NULL, 16);
-		return 0;
-	}
+		e.key	= name;
+		e.data	= value;
+		hsearch_r(e, ENTER, &ep, &env_htab);
+		free(value);
+		if (!ep) {
+			printf("## Error inserting \"%s\" variable, errno=%d\n",
+				name, errno);
+			return 1;
+		}
+
+		/*
+		 * Some variables should be updated when the corresponding
+		 * entry in the environment is changed
+		 */
+		if (strcmp(argv[1], "loadaddr") == 0) {
+			load_addr = simple_strtoul(argv[2], NULL, 16);
+			return 0;
+		}
 #if defined(CONFIG_CMD_NET)
-	else if (strcmp(argv[1], "bootfile") == 0) {
-		copy_filename(BootFile, argv[2], sizeof(BootFile));
-		return 0;
-	}
+		else if (strcmp(argv[1], "bootfile") == 0) {
+			copy_filename(BootFile, argv[2], sizeof(BootFile));
+			return 0;
+		}
 #endif
+	}
 	return 0;
 }
 
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 23cf241..a4cd179 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -368,6 +368,7 @@ int fw_env_write(char *name, char *value)
 	int len;
 	char *env, *nxt;
 	char *oldval = NULL;
+	int deleting, creating, overwriting;
 
 	/*
 	 * search if variable with this name already exists
@@ -385,10 +386,11 @@ int fw_env_write(char *name, char *value)
 			break;
 	}
 
-	/*
-	 * Delete any existing definition
-	 */
-	if (oldval) {
+	deleting = (oldval && !(value && strlen(value)));
+	creating = (!oldval && (value && strlen(value)));
+	overwriting = (oldval && (value && strlen(value)));
+
+	if (deleting || overwriting) {
 #ifndef CONFIG_ENV_OVERWRITE
 		/*
 		 * Ethernet Address and serial# can be set only once
@@ -419,39 +421,38 @@ int fw_env_write(char *name, char *value)
 		*++env = '\0';
 	}
 
-	/* Delete only ? */
-	if (!value || !strlen(value))
-		return 0;
-
-	/*
-	 * Append new definition at the end
-	 */
-	for (env = environment.data; *env || *(env + 1); ++env);
-	if (env > environment.data)
-		++env;
-	/*
-	 * Overflow when:
-	 * "name" + "=" + "val" +"\0\0"  > CUR_ENVSIZE - (env-environment)
-	 */
-	len = strlen (name) + 2;
-	/* add '=' for first arg, ' ' for all others */
-	len += strlen(value) + 1;
+	if (creating || overwriting) {
+		/*
+		 * Append new definition at the end
+		 */
+		for (env = environment.data; *env || *(env + 1); ++env)
+			continue;
+		if (env > environment.data)
+			++env;
+		/*
+		 * Overflow when:
+		 * "name" + "=" + "val" +"\0\0"  > CUR_ENVSIZE - (env)
+		 */
+		len = strlen(name) + 2;
+		/* add '=' for first arg, ' ' for all others */
+		len += strlen(value) + 1;
 
-	if (len > (&environment.data[ENV_SIZE] - env)) {
-		fprintf (stderr,
-			"Error: environment overflow, \"%s\" deleted\n",
-			name);
-		return -1;
-	}
+		if (len > (&environment.data[ENV_SIZE] - env)) {
+			fprintf(stderr,
+				"Error: environment overflow, \"%s\" deleted\n",
+				name);
+			return -1;
+		}
 
-	while ((*env = *name++) != '\0')
-		env++;
-	*env = '=';
-	while ((*++env = *value++) != '\0')
-		;
+		while ((*env = *name++) != '\0')
+			env++;
+		*env = '=';
+		while ((*++env = *value++) != '\0')
+			continue;
 
-	/* end is marked with double '\0' */
-	*++env = '\0';
+		/* end is marked with double '\0' */
+		*++env = '\0';
+	}
 
 	return 0;
 }
-- 
1.7.11.5



More information about the U-Boot mailing list