[U-Boot] [PATCH v2 0/7] tools: env: simplify argument parsing

[U-Boot] [PATCH v2 0/7] tools: env: simplify argument parsing

[Andreas Fenkart]

In it's current state paramter parsing is quite hard to
understand since it happens in two places. One is using getopt
at the beginning of main, the second is using adhoc parsing 
where the order of arguments is important.
This patch will parse arguments only in one place using getopt
and store the parsed flags in a global struct.

v2:
- rebased on top of current master

Andreas Fenkart (7):
  tools: env validate: pass values as 0-based array
  tools: env: make parse_aes_key stateless
  tools: env: introduce setenv/printenv argument structs
  tools: env: parse aes key / suppress flag into argument struct
  tools: env: shift optind arguments and fix argument indices
  tools: env: factor out parse_common_args
  tools: env: update usage strings

 common/env_flags.c      |  14 +--
 include/env_flags.h     |   2 +-
 tools/env/fw_env.c      | 127 ++++++------------------
 tools/env/fw_env.h      |  22 +++++
 tools/env/fw_env_main.c | 250 ++++++++++++++++++++++++++++++++++--------------
 5 files changed, 238 insertions(+), 177 deletions(-)

-- 
2.6.2

[U-Boot] [PATCH v2 1/7] tools: env validate: pass values as 0-based array

[Andreas Fenkart]

passing argv/argc can produce off-by-one errors

Signed-off-by: Andreas Fenkart <andreas.fenkart@dev.digitalstrom.org>
---
 common/env_flags.c  | 14 +++++++-------
 include/env_flags.h |  2 +-
 tools/env/fw_env.c  | 11 +++++++----
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/common/env_flags.c b/common/env_flags.c
index e682d85..529e371 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -373,21 +373,21 @@ int env_flags_validate_varaccess(const char *name, int check_mask)
 /*
  * Validate the parameters to "env set" directly
  */
-int env_flags_validate_env_set_params(int argc, char * const argv[])
+int env_flags_validate_env_set_params(char *name, char * const val[], int count)
 {
-	if ((argc >= 3) && argv[2] != NULL) {
-		enum env_flags_vartype type = env_flags_get_type(argv[1]);
+	if ((count >= 1) && val[0] != NULL) {
+		enum env_flags_vartype type = env_flags_get_type(name);
 
 		/*
 		 * we don't currently check types that need more than
 		 * one argument
 		 */
-		if (type != env_flags_vartype_string && argc > 3) {
-			printf("## Error: too many parameters for setting "
-				"\"%s\"\n", argv[1]);
+		if (type != env_flags_vartype_string && count > 1) {
+			printf("## Error: too many parameters for setting \"%s\"\n",
+			       name);
 			return -1;
 		}
-		return env_flags_validate_type(argv[1], argv[2]);
+		return env_flags_validate_type(name, val[0]);
 	}
 	/* ok */
 	return 0;
diff --git a/include/env_flags.h b/include/env_flags.h
index 8823fb9..9e87e1b 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -143,7 +143,7 @@ int env_flags_validate_varaccess(const char *name, int check_mask);
 /*
  * Validate the parameters passed to "env set" for type compliance
  */
-int env_flags_validate_env_set_params(int argc, char * const argv[]);
+int env_flags_validate_env_set_params(char *name, char *const val[], int count);
 
 #else /* !USE_HOSTCC */
 
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index ba11f77..22507f6 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -497,8 +497,9 @@ int fw_setenv(int argc, char *argv[])
 {
 	int i, rc;
 	size_t len;
-	char *name;
+	char *name, **valv;
 	char *value = NULL;
+	int valc;
 
 #ifdef CONFIG_FILE
 	if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
@@ -542,13 +543,15 @@ int fw_setenv(int argc, char *argv[])
 	}
 
 	name = argv[1];
+	valv = argv + 2;
+	valc = argc - 2;
 
-	if (env_flags_validate_env_set_params(argc, argv) < 0)
+	if (env_flags_validate_env_set_params(name, valv, valc) < 0)
 		return 1;
 
 	len = 0;
-	for (i = 2; i < argc; ++i) {
-		char *val = argv[i];
+	for (i = 0; i < valc; ++i) {
+		char *val = valv[i];
 		size_t val_len = strlen(val);
 
 		if (value)
-- 
2.6.2

[U-Boot] [PATCH v2 2/7] tools: env: make parse_aes_key stateless

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenkart@dev.digitalstrom.org>
---
 tools/env/fw_env.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 22507f6..5b76b74 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -207,7 +207,7 @@ char *fw_getdefenv(char *name)
 	return NULL;
 }
 
-static int parse_aes_key(char *key)
+static int parse_aes_key(char *key, uint8_t *bin_key)
 {
 	char tmp[5] = { '0', 'x', 0, 0, 0 };
 	unsigned long ul;
@@ -229,11 +229,9 @@ static int parse_aes_key(char *key)
 				"## Error: '-a' option requires valid AES key\n");
 			return -1;
 		}
-		aes_key[i] = ul & 0xff;
+		bin_key[i] = ul & 0xff;
 		key += 2;
 	}
-	aes_flag = 1;
-
 	return 0;
 }
 
@@ -266,9 +264,10 @@ int fw_printenv (int argc, char *argv[])
 				"## Error: '-a' option requires AES key\n");
 			return -1;
 		}
-		rc = parse_aes_key(argv[2]);
+		rc = parse_aes_key(argv[2], aes_key);
 		if (rc)
 			return rc;
+		aes_flag = 1;
 		argv += 2;
 		argc -= 2;
 	}
@@ -525,9 +524,10 @@ int fw_setenv(int argc, char *argv[])
 				"## Error: '-a' option requires AES key\n");
 			return -1;
 		}
-		rc = parse_aes_key(argv[2]);
+		rc = parse_aes_key(argv[2], aes_key);
 		if (rc)
 			return rc;
+		aes_flag = 1;
 		argv += 2;
 		argc -= 2;
 	}
-- 
2.6.2

[U-Boot] [PATCH v2 3/7] tools: env: introduce setenv/printenv argument structs

[Andreas Fenkart]

goal is to use getopt for all argument parsing instead of adhoc
parsing in fw_getenv/fw_setenv functions

Signed-off-by: Andreas Fenkart <andreas.fenkart@dev.digitalstrom.org>
---
 tools/env/fw_env.h      |   9 ++++
 tools/env/fw_env_main.c | 113 ++++++++++++++++++++++++++++++++----------------
 2 files changed, 84 insertions(+), 38 deletions(-)

diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 60c0517..1a02c46 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -54,6 +54,15 @@
 	"bootm"
 #endif
 
+struct printenv_args {
+};
+extern struct printenv_args printenv_args;
+
+struct setenv_args {
+	char *script_file;
+};
+extern struct setenv_args setenv_args;
+
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);
 extern int fw_setenv  (int argc, char *argv[]);
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 234c061..0c9f918 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -45,6 +45,9 @@ static struct option long_options[] = {
 	{NULL, 0, NULL, 0}
 };
 
+struct printenv_args printenv_args;
+struct setenv_args setenv_args;
+
 void usage(void)
 {
 
@@ -77,31 +80,9 @@ void usage(void)
 	);
 }
 
-int main(int argc, char *argv[])
+int parse_printenv_args(int argc, char *argv[])
 {
-	char *p;
-	char *cmdname = *argv;
-	char *script_file = NULL;
 	int c;
-	const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
-	int lockfd = -1;
-	int retval = EXIT_SUCCESS;
-
-	lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
-	if (-1 == lockfd) {
-		fprintf(stderr, "Error opening lock file %s\n", lockname);
-		return EXIT_FAILURE;
-	}
-
-	if (-1 == flock(lockfd, LOCK_EX)) {
-		fprintf(stderr, "Error locking file %s\n", lockname);
-		close(lockfd);
-		return EXIT_FAILURE;
-	}
-
-	if ((p = strrchr (cmdname, '/')) != NULL) {
-		cmdname = p + 1;
-	}
 
 	while ((c = getopt_long (argc, argv, "a:c:ns:h",
 		long_options, NULL)) != EOF) {
@@ -115,40 +96,96 @@ int main(int argc, char *argv[])
 		case 'n':
 			/* handled in fw_printenv */
 			break;
+		case 'h':
+			usage();
+			exit(EXIT_SUCCESS);
+			break;
+		default: /* '?' */
+			usage();
+			exit(EXIT_FAILURE);
+			break;
+		}
+	}
+	return 0;
+}
+
+int parse_setenv_args(int argc, char *argv[])
+{
+	int c;
+
+	while ((c = getopt_long (argc, argv, "a:c:ns:h",
+		long_options, NULL)) != EOF) {
+		switch (c) {
+		case 'a':
+			/* AES key, handled later */
+			break;
+		case 'c':
+			/* handled later */
+			break;
 		case 's':
-			script_file = optarg;
+			setenv_args.script_file = optarg;
 			break;
 		case 'h':
 			usage();
-			goto exit;
+			exit(EXIT_SUCCESS);
+			break;
 		default: /* '?' */
-			fprintf(stderr, "Try `%s --help' for more information."
-				"\n", cmdname);
-			retval = EXIT_FAILURE;
-			goto exit;
+			usage();
+			exit(EXIT_FAILURE);
+			break;
 		}
 	}
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	char *cmdname = *argv;
+	const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
+	int lockfd = -1;
+	int retval = EXIT_SUCCESS;
+
+	if (strrchr(cmdname, '/') != NULL)
+		cmdname = strrchr(cmdname, '/') + 1;
+
+	if (strcmp(cmdname, CMD_PRINTENV) == 0) {
+		if (parse_printenv_args(argc, argv))
+			exit(EXIT_FAILURE);
+	} else if (strcmp(cmdname, CMD_SETENV) == 0) {
+		if (parse_setenv_args(argc, argv))
+			exit(EXIT_FAILURE);
+	} else {
+		fprintf(stderr,
+			"Identity crisis - may be called as `%s' or as `%s' but not as `%s'\n",
+			CMD_PRINTENV, CMD_SETENV, cmdname);
+		exit(EXIT_FAILURE);
+	}
+
+	lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+	if (-1 == lockfd) {
+		fprintf(stderr, "Error opening lock file %s\n", lockname);
+		return EXIT_FAILURE;
+	}
+
+	if (-1 == flock(lockfd, LOCK_EX)) {
+		fprintf(stderr, "Error locking file %s\n", lockname);
+		close(lockfd);
+		return EXIT_FAILURE;
+	}
 
 	if (strcmp(cmdname, CMD_PRINTENV) == 0) {
 		if (fw_printenv(argc, argv) != 0)
 			retval = EXIT_FAILURE;
 	} else if (strcmp(cmdname, CMD_SETENV) == 0) {
-		if (!script_file) {
+		if (!setenv_args.script_file) {
 			if (fw_setenv(argc, argv) != 0)
 				retval = EXIT_FAILURE;
 		} else {
-			if (fw_parse_script(script_file) != 0)
+			if (fw_parse_script(setenv_args.script_file) != 0)
 				retval = EXIT_FAILURE;
 		}
-	} else {
-		fprintf(stderr,
-			"Identity crisis - may be called as `" CMD_PRINTENV
-			"' or as `" CMD_SETENV "' but not as `%s'\n",
-			cmdname);
-		retval = EXIT_FAILURE;
 	}
 
-exit:
 	flock(lockfd, LOCK_UN);
 	close(lockfd);
 	return retval;
-- 
2.6.2

[U-Boot] [PATCH v2 4/7] tools: env: parse aes key / suppress flag into argument struct

[Andreas Fenkart]

disabled original parsing, but not yet removed since the
argument indexing needs to be fixed

Signed-off-by: Andreas Fenkart <andreas.fenkart@dev.digitalstrom.org>
---
 tools/env/fw_env.c      | 64 ++++++++++---------------------------------------
 tools/env/fw_env.h      | 13 ++++++++++
 tools/env/fw_env_main.c | 31 ++++++++++++++++++++----
 3 files changed, 51 insertions(+), 57 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 5b76b74..2cfff02 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -33,8 +33,6 @@
 
 #include "fw_env.h"
 
-#include <aes.h>
-
 #define DIV_ROUND_UP(n, d)	(((n) + (d) - 1) / (d))
 
 #define WHITESPACE(c) ((c == '\t') || (c == ' '))
@@ -104,9 +102,6 @@ static struct environment environment = {
 	.flag_scheme = FLAG_NONE,
 };
 
-/* Is AES encryption used? */
-static int aes_flag;
-static uint8_t aes_key[AES_KEY_LENGTH] = { 0 };
 static int env_aes_cbc_crypt(char *data, const int enc);
 
 static int HaveRedundEnv = 0;
@@ -124,7 +119,6 @@ static int parse_config (void);
 
 #if defined(CONFIG_FILE)
 static int get_config (char *);
-static char *config_file = CONFIG_FILE;
 #endif
 static inline ulong getenvsize (void)
 {
@@ -133,7 +127,7 @@ static inline ulong getenvsize (void)
 	if (HaveRedundEnv)
 		rc -= sizeof (char);
 
-	if (aes_flag)
+	if (common_args.aes_flag)
 		rc &= ~(AES_KEY_LENGTH - 1);
 
 	return rc;
@@ -207,7 +201,7 @@ char *fw_getdefenv(char *name)
 	return NULL;
 }
 
-static int parse_aes_key(char *key, uint8_t *bin_key)
+int parse_aes_key(char *key, uint8_t *bin_key)
 {
 	char tmp[5] = { '0', 'x', 0, 0, 0 };
 	unsigned long ul;
@@ -242,32 +236,16 @@ static int parse_aes_key(char *key, uint8_t *bin_key)
 int fw_printenv (int argc, char *argv[])
 {
 	char *env, *nxt;
-	int i, n_flag;
-	int rc = 0;
+	int i, rc = 0;
 
 #ifdef CONFIG_FILE
 	if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-		if (argc < 3) {
-			fprintf(stderr,
-				"## Error: '-c' option requires the config file to use\n");
-			return -1;
-		}
-		config_file = argv[2];
 		argv += 2;
 		argc -= 2;
 	}
 #endif
 
 	if (argc >= 2 && strcmp(argv[1], "-a") == 0) {
-		if (argc < 3) {
-			fprintf(stderr,
-				"## Error: '-a' option requires AES key\n");
-			return -1;
-		}
-		rc = parse_aes_key(argv[2], aes_key);
-		if (rc)
-			return rc;
-		aes_flag = 1;
 		argv += 2;
 		argc -= 2;
 	}
@@ -291,7 +269,6 @@ int fw_printenv (int argc, char *argv[])
 	}
 
 	if (strcmp (argv[1], "-n") == 0) {
-		n_flag = 1;
 		++argv;
 		--argc;
 		if (argc != 2) {
@@ -299,8 +276,6 @@ int fw_printenv (int argc, char *argv[])
 				"`-n' option requires exactly one argument\n");
 			return -1;
 		}
-	} else {
-		n_flag = 0;
 	}
 
 	for (i = 1; i < argc; ++i) {	/* print single env variables   */
@@ -318,7 +293,7 @@ int fw_printenv (int argc, char *argv[])
 			}
 			val = envmatch (name, env);
 			if (val) {
-				if (!n_flag) {
+				if (!printenv_args.name_suppress) {
 					fputs (name, stdout);
 					putc ('=', stdout);
 				}
@@ -338,7 +313,7 @@ int fw_printenv (int argc, char *argv[])
 int fw_env_close(void)
 {
 	int ret;
-	if (aes_flag) {
+	if (common_args.aes_flag) {
 		ret = env_aes_cbc_crypt(environment.data, 1);
 		if (ret) {
 			fprintf(stderr,
@@ -494,7 +469,7 @@ int fw_env_write(char *name, char *value)
  */
 int fw_setenv(int argc, char *argv[])
 {
-	int i, rc;
+	int i;
 	size_t len;
 	char *name, **valv;
 	char *value = NULL;
@@ -502,12 +477,6 @@ int fw_setenv(int argc, char *argv[])
 
 #ifdef CONFIG_FILE
 	if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-		if (argc < 3) {
-			fprintf(stderr,
-				"## Error: '-c' option requires the config file to use\n");
-			return -1;
-		}
-		config_file = argv[2];
 		argv += 2;
 		argc -= 2;
 	}
@@ -519,15 +488,6 @@ int fw_setenv(int argc, char *argv[])
 	}
 
 	if (strcmp(argv[1], "-a") == 0) {
-		if (argc < 3) {
-			fprintf(stderr,
-				"## Error: '-a' option requires AES key\n");
-			return -1;
-		}
-		rc = parse_aes_key(argv[2], aes_key);
-		if (rc)
-			return rc;
-		aes_flag = 1;
 		argv += 2;
 		argc -= 2;
 	}
@@ -1025,7 +985,7 @@ static int env_aes_cbc_crypt(char *payload, const int enc)
 	uint32_t aes_blocks;
 
 	/* First we expand the key. */
-	aes_expand_key(aes_key, key_exp);
+	aes_expand_key(common_args.aes_key, key_exp);
 
 	/* Calculate the number of AES blocks to encrypt. */
 	aes_blocks = DIV_ROUND_UP(len, AES_KEY_LENGTH);
@@ -1253,7 +1213,7 @@ int fw_env_open(void)
 
 	crc0 = crc32 (0, (uint8_t *) environment.data, ENV_SIZE);
 
-	if (aes_flag) {
+	if (common_args.aes_flag) {
 		ret = env_aes_cbc_crypt(environment.data, 0);
 		if (ret)
 			return ret;
@@ -1310,7 +1270,7 @@ int fw_env_open(void)
 
 		crc1 = crc32 (0, (uint8_t *) redundant->data, ENV_SIZE);
 
-		if (aes_flag) {
+		if (common_args.aes_flag) {
 			ret = env_aes_cbc_crypt(redundant->data, 0);
 			if (ret)
 				return ret;
@@ -1394,9 +1354,9 @@ static int parse_config ()
 
 #if defined(CONFIG_FILE)
 	/* Fills in DEVNAME(), ENVSIZE(), DEVESIZE(). Or don't. */
-	if (get_config (config_file)) {
-		fprintf (stderr,
-			"Cannot parse config file '%s': %s\n", config_file, strerror (errno));
+	if (get_config(common_args.config_file)) {
+		fprintf(stderr, "Cannot parse config file '%s': %m\n",
+			common_args.config_file);
 		return -1;
 	}
 #else
diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index 1a02c46..696eace 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -5,6 +5,9 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#include <aes.h>
+#include <stdint.h>
+
 /* Pull in the current config to define the default environment */
 #include <linux/kconfig.h>
 
@@ -54,7 +57,15 @@
 	"bootm"
 #endif
 
+struct common_args {
+	char *config_file;
+	uint8_t aes_key[AES_KEY_LENGTH];
+	int aes_flag; /* Is AES encryption used? */
+};
+extern struct common_args common_args;
+
 struct printenv_args {
+	int name_suppress;
 };
 extern struct printenv_args printenv_args;
 
@@ -63,6 +74,8 @@ struct setenv_args {
 };
 extern struct setenv_args setenv_args;
 
+int parse_aes_key(char *key, uint8_t *bin_key);
+
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);
 extern int fw_setenv  (int argc, char *argv[]);
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index 0c9f918..b68f1bf 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -45,6 +45,7 @@ static struct option long_options[] = {
 	{NULL, 0, NULL, 0}
 };
 
+struct common_args common_args;
 struct printenv_args printenv_args;
 struct setenv_args setenv_args;
 
@@ -84,17 +85,27 @@ int parse_printenv_args(int argc, char *argv[])
 {
 	int c;
 
+#ifdef CONFIG_FILE
+	common_args.config_file = CONFIG_FILE;
+#endif
+
 	while ((c = getopt_long (argc, argv, "a:c:ns:h",
 		long_options, NULL)) != EOF) {
 		switch (c) {
 		case 'a':
-			/* AES key, handled later */
+			if (parse_aes_key(optarg, common_args.aes_key)) {
+				fprintf(stderr, "AES key parse error\n");
+				return EXIT_FAILURE;
+			}
+			common_args.aes_flag = 1;
 			break;
+#ifdef CONFIG_FILE
 		case 'c':
-			/* handled later */
+			common_args.config_file = optarg;
 			break;
+#endif
 		case 'n':
-			/* handled in fw_printenv */
+			printenv_args.name_suppress = 1;
 			break;
 		case 'h':
 			usage();
@@ -113,15 +124,25 @@ int parse_setenv_args(int argc, char *argv[])
 {
 	int c;
 
+#ifdef CONFIG_FILE
+	common_args.config_file = CONFIG_FILE;
+#endif
+
 	while ((c = getopt_long (argc, argv, "a:c:ns:h",
 		long_options, NULL)) != EOF) {
 		switch (c) {
 		case 'a':
-			/* AES key, handled later */
+			if (parse_aes_key(optarg, common_args.aes_key)) {
+				fprintf(stderr, "AES key parse error\n");
+				return EXIT_FAILURE;
+			}
+			common_args.aes_flag = 1;
 			break;
+#ifdef CONFIG_FILE
 		case 'c':
-			/* handled later */
+			common_args.config_file = optarg;
 			break;
+#endif
 		case 's':
 			setenv_args.script_file = optarg;
 			break;
-- 
2.6.2

[U-Boot] [PATCH v2 5/7] tools: env: shift optind arguments and fix argument indices

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenkart@dev.digitalstrom.org>
---
 tools/env/fw_env.c      | 54 ++++++++++---------------------------------------
 tools/env/fw_env_main.c |  4 ++++
 2 files changed, 15 insertions(+), 43 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 2cfff02..b1cf9ae 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -238,22 +238,10 @@ int fw_printenv (int argc, char *argv[])
 	char *env, *nxt;
 	int i, rc = 0;
 
-#ifdef CONFIG_FILE
-	if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-		argv += 2;
-		argc -= 2;
-	}
-#endif
-
-	if (argc >= 2 && strcmp(argv[1], "-a") == 0) {
-		argv += 2;
-		argc -= 2;
-	}
-
 	if (fw_env_open())
 		return -1;
 
-	if (argc == 1) {		/* Print all env variables  */
+	if (argc == 0) {		/* Print all env variables  */
 		for (env = environment.data; *env; env = nxt + 1) {
 			for (nxt = env; *nxt; ++nxt) {
 				if (nxt >= &environment.data[ENV_SIZE]) {
@@ -268,17 +256,13 @@ int fw_printenv (int argc, char *argv[])
 		return 0;
 	}
 
-	if (strcmp (argv[1], "-n") == 0) {
-		++argv;
-		--argc;
-		if (argc != 2) {
-			fprintf (stderr, "## Error: "
-				"`-n' option requires exactly one argument\n");
-			return -1;
-		}
+	if (printenv_args.name_suppress && argc != 1) {
+		fprintf(stderr,
+			"## Error: `-n' option requires exactly one argument\n");
+		return -1;
 	}
 
-	for (i = 1; i < argc; ++i) {	/* print single env variables   */
+	for (i = 0; i < argc; ++i) {	/* print single env variables   */
 		char *name = argv[i];
 		char *val = NULL;
 
@@ -475,24 +459,8 @@ int fw_setenv(int argc, char *argv[])
 	char *value = NULL;
 	int valc;
 
-#ifdef CONFIG_FILE
-	if (argc >= 2 && strcmp(argv[1], "-c") == 0) {
-		argv += 2;
-		argc -= 2;
-	}
-#endif
-
-	if (argc < 2) {
-		errno = EINVAL;
-		return -1;
-	}
-
-	if (strcmp(argv[1], "-a") == 0) {
-		argv += 2;
-		argc -= 2;
-	}
-
-	if (argc < 2) {
+	if (argc < 1) {
+		fprintf(stderr, "## Error: variable name missing\n");
 		errno = EINVAL;
 		return -1;
 	}
@@ -502,9 +470,9 @@ int fw_setenv(int argc, char *argv[])
 		return -1;
 	}
 
-	name = argv[1];
-	valv = argv + 2;
-	valc = argc - 2;
+	name = argv[0];
+	valv = argv + 1;
+	valc = argc - 1;
 
 	if (env_flags_validate_env_set_params(name, valv, valc) < 0)
 		return 1;
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index b68f1bf..daf4688 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -182,6 +182,10 @@ int main(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	/* shift parsed flags, jump to non-option arguments */
+	argc -= optind;
+	argv += optind;
+
 	lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
 	if (-1 == lockfd) {
 		fprintf(stderr, "Error opening lock file %s\n", lockname);
-- 
2.6.2

[U-Boot] [PATCH v2 6/7] tools: env: factor out parse_common_args

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenkart@dev.digitalstrom.org>
---
 tools/env/fw_env_main.c | 63 +++++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 25 deletions(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index daf4688..e7ba95e 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -81,7 +81,7 @@ void usage(void)
 	);
 }
 
-int parse_printenv_args(int argc, char *argv[])
+static void parse_common_args(int argc, char *argv[])
 {
 	int c;
 
@@ -89,27 +89,53 @@ int parse_printenv_args(int argc, char *argv[])
 	common_args.config_file = CONFIG_FILE;
 #endif
 
-	while ((c = getopt_long (argc, argv, "a:c:ns:h",
-		long_options, NULL)) != EOF) {
+	while ((c = getopt_long(argc, argv, ":a:c:h", long_options, NULL)) !=
+	       EOF) {
 		switch (c) {
 		case 'a':
 			if (parse_aes_key(optarg, common_args.aes_key)) {
 				fprintf(stderr, "AES key parse error\n");
-				return EXIT_FAILURE;
+				exit(EXIT_FAILURE);
 			}
 			common_args.aes_flag = 1;
 			break;
 #ifdef CONFIG_FILE
 		case 'c':
+			printf("config file\n");
 			common_args.config_file = optarg;
 			break;
 #endif
+		case 'h':
+			usage();
+			exit(EXIT_SUCCESS);
+			break;
+		default:
+			/* ignore unknown options */
+			break;
+		}
+	}
+
+	/* Reset getopt for the next pass. */
+	opterr = 1;
+	optind = 1;
+}
+
+int parse_printenv_args(int argc, char *argv[])
+{
+	int c;
+
+	parse_common_args(argc, argv);
+
+	while ((c = getopt_long(argc, argv, "a:c:ns:h", long_options, NULL)) !=
+	       EOF) {
+		switch (c) {
 		case 'n':
 			printenv_args.name_suppress = 1;
 			break;
+		case 'a':
+		case 'c':
 		case 'h':
-			usage();
-			exit(EXIT_SUCCESS);
+			/* ignore common options */
 			break;
 		default: /* '?' */
 			usage();
@@ -124,31 +150,18 @@ int parse_setenv_args(int argc, char *argv[])
 {
 	int c;
 
-#ifdef CONFIG_FILE
-	common_args.config_file = CONFIG_FILE;
-#endif
+	parse_common_args(argc, argv);
 
-	while ((c = getopt_long (argc, argv, "a:c:ns:h",
-		long_options, NULL)) != EOF) {
+	while ((c = getopt_long(argc, argv, "a:c:ns:h", long_options, NULL)) !=
+	       EOF) {
 		switch (c) {
-		case 'a':
-			if (parse_aes_key(optarg, common_args.aes_key)) {
-				fprintf(stderr, "AES key parse error\n");
-				return EXIT_FAILURE;
-			}
-			common_args.aes_flag = 1;
-			break;
-#ifdef CONFIG_FILE
-		case 'c':
-			common_args.config_file = optarg;
-			break;
-#endif
 		case 's':
 			setenv_args.script_file = optarg;
 			break;
+		case 'a':
+		case 'c':
 		case 'h':
-			usage();
-			exit(EXIT_SUCCESS);
+			/* ignore common options */
 			break;
 		default: /* '?' */
 			usage();
-- 
2.6.2

[U-Boot] [PATCH v2 7/7] tools: env: update usage strings

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <andreas.fenkart@dev.digitalstrom.org>
---
 tools/env/fw_env_main.c | 117 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 75 insertions(+), 42 deletions(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index e7ba95e..79273ce 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -36,12 +36,16 @@
 #include <unistd.h>
 #include "fw_env.h"
 
-#define	CMD_PRINTENV	"fw_printenv"
+#define CMD_PRINTENV	"fw_printenv"
 #define CMD_SETENV	"fw_setenv"
+static int do_printenv;
 
 static struct option long_options[] = {
-	{"script", required_argument, NULL, 's'},
+	{"aes", required_argument, NULL, 'a'},
+	{"config", required_argument, NULL, 'c'},
 	{"help", no_argument, NULL, 'h'},
+	{"script", required_argument, NULL, 's'},
+	{"noheader", required_argument, NULL, 'n'},
 	{NULL, 0, NULL, 0}
 };
 
@@ -49,36 +53,58 @@ struct common_args common_args;
 struct printenv_args printenv_args;
 struct setenv_args setenv_args;
 
-void usage(void)
+void usage_printenv(void)
 {
 
-	fprintf(stderr, "fw_printenv/fw_setenv, "
-		"a command line interface to U-Boot environment\n\n"
-#ifndef CONFIG_FILE
-		"usage:\tfw_printenv [-a key] [-n] [variable name]\n"
-		"\tfw_setenv [-a key] [variable name] [variable value]\n"
-#else
-		"usage:\tfw_printenv [-c /my/fw_env.config] [-a key] [-n] [variable name]\n"
-		"\tfw_setenv [-c /my/fw_env.config] [-a key] [variable name] [variable value]\n"
+	fprintf(stderr,
+		"Usage: fw_printenv [OPTIONS]... [VARIABLE]...\n"
+		"Print variables from U-Boot environment\n"
+		"\n"
+		" -h, --help           print this help.\n"
+#ifdef CONFIG_ENV_AES
+		" -a, --aes            aes key to access environment\n"
 #endif
-		"\tfw_setenv -s [ file ]\n"
-		"\tfw_setenv -s - < [ file ]\n\n"
-		"The file passed as argument contains only pairs "
-		"name / value\n"
-		"Example:\n"
-		"# Any line starting with # is treated as comment\n"
+#ifdef CONFIG_FILE
+		" -c, --config         configuration file, default:" CONFIG_FILE "\n"
+#endif
+		" -n, --noheader       do not repeat variable name in output\n"
+		"\n");
+}
+
+void usage_setenv(void)
+{
+	fprintf(stderr,
+		"Usage: fw_setenv [OPTIONS]... [VARIABLE]...\n"
+		"Modify variables in U-Boot environment\n"
+		"\n"
+		" -h, --help           print this help.\n"
+#ifdef CONFIG_ENV_AES
+		" -a, --aes            aes key to access environment\n"
+#endif
+#ifdef CONFIG_FILE
+		" -c, --config         configuration file, default:" CONFIG_FILE "\n"
+#endif
+		" -s, --script         batch mode to minimize writes\n"
+		"\n"
+		"Examples:\n"
+		"  fw_setenv foo bar   set variable foo equal bar\n"
+		"  fw_setenv foo       clear variable foo\n"
+		"  fw_setenv --script file run batch script\n"
 		"\n"
-		"\t      netdev         eth0\n"
-		"\t      kernel_addr    400000\n"
-		"\t      var1\n"
-		"\t      var2          The quick brown fox jumps over the "
-		"lazy dog\n"
+		"Script Syntax:\n"
+		"  key [space] value\n"
+		"  lines starting with '#' are treated as commment\n"
 		"\n"
-		"A variable without value will be dropped. It is possible\n"
-		"to put any number of spaces between the fields, but any\n"
-		"space inside the value is treated as part of the value "
-		"itself.\n\n"
-	);
+		"  A variable without value will be deleted. Any number of spaces are\n"
+		"  allowed between key and value. Space inside of the value is treated\n"
+		"  as part of the value itself.\n"
+		"\n"
+		"Script Example:\n"
+		"  netdev         eth0\n"
+		"  kernel_addr    400000\n"
+		"  foo            empty empty empty    empty empty empty\n"
+		"  bar\n"
+		"\n");
 }
 
 static void parse_common_args(int argc, char *argv[])
@@ -106,7 +132,7 @@ static void parse_common_args(int argc, char *argv[])
 			break;
 #endif
 		case 'h':
-			usage();
+			do_printenv ? usage_printenv() : usage_setenv();
 			exit(EXIT_SUCCESS);
 			break;
 		default:
@@ -138,7 +164,7 @@ int parse_printenv_args(int argc, char *argv[])
 			/* ignore common options */
 			break;
 		default: /* '?' */
-			usage();
+			usage_printenv();
 			exit(EXIT_FAILURE);
 			break;
 		}
@@ -164,7 +190,7 @@ int parse_setenv_args(int argc, char *argv[])
 			/* ignore common options */
 			break;
 		default: /* '?' */
-			usage();
+			usage_setenv();
 			exit(EXIT_FAILURE);
 			break;
 		}
@@ -174,27 +200,34 @@ int parse_setenv_args(int argc, char *argv[])
 
 int main(int argc, char *argv[])
 {
-	char *cmdname = *argv;
 	const char *lockname = "/var/lock/" CMD_PRINTENV ".lock";
 	int lockfd = -1;
 	int retval = EXIT_SUCCESS;
+	char *_cmdname;
 
-	if (strrchr(cmdname, '/') != NULL)
-		cmdname = strrchr(cmdname, '/') + 1;
+	_cmdname = *argv;
+	if (strrchr(_cmdname, '/') != NULL)
+		_cmdname = strrchr(_cmdname, '/') + 1;
 
-	if (strcmp(cmdname, CMD_PRINTENV) == 0) {
-		if (parse_printenv_args(argc, argv))
-			exit(EXIT_FAILURE);
-	} else if (strcmp(cmdname, CMD_SETENV) == 0) {
-		if (parse_setenv_args(argc, argv))
-			exit(EXIT_FAILURE);
+	if (strcmp(_cmdname, CMD_PRINTENV) == 0) {
+		do_printenv = 1;
+	} else if (strcmp(_cmdname, CMD_SETENV) == 0) {
+		do_printenv = 0;
 	} else {
 		fprintf(stderr,
 			"Identity crisis - may be called as `%s' or as `%s' but not as `%s'\n",
-			CMD_PRINTENV, CMD_SETENV, cmdname);
+			CMD_PRINTENV, CMD_SETENV, _cmdname);
 		exit(EXIT_FAILURE);
 	}
 
+	if (do_printenv) {
+		if (parse_printenv_args(argc, argv))
+			exit(EXIT_FAILURE);
+	} else {
+		if (parse_setenv_args(argc, argv))
+			exit(EXIT_FAILURE);
+	}
+
 	/* shift parsed flags, jump to non-option arguments */
 	argc -= optind;
 	argv += optind;
@@ -211,10 +244,10 @@ int main(int argc, char *argv[])
 		return EXIT_FAILURE;
 	}
 
-	if (strcmp(cmdname, CMD_PRINTENV) == 0) {
+	if (do_printenv) {
 		if (fw_printenv(argc, argv) != 0)
 			retval = EXIT_FAILURE;
-	} else if (strcmp(cmdname, CMD_SETENV) == 0) {
+	} else {
 		if (!setenv_args.script_file) {
 			if (fw_setenv(argc, argv) != 0)
 				retval = EXIT_FAILURE;
-- 
2.6.2