[RFC PATCH v2 10/17] cmd: nvedit: Use getopt() in env export

Simon Glass sjg at chromium.org
Wed May 20 01:31:36 CEST 2026


This is one of the gnarliest hand-rolled parsers in nvedit.c

The code walks each -prefixed argv element by hand, opens an inner
while (*++arg) to split grouped flags, and tracks a fmt counter to
catch a second -b / -c / -t

It also adds a 'goto NXTARG' that jumps over the inner loop after
consuming the argument to -s SIZE, since the rest of the current
argv element no longer makes sense once -s has eaten the next slot.

getopt() collapses all of that. The mutex check for the format flags
stays in the case arms (it is a per-command rule, not an option-parsing
rule), the -s argument arrives in gs.arg, and the trailing positional
list is read straight out of gs.args + gs.index with gs.nonopts as the
count.

Permutation now lets, for example, 'env export 1000000 -t -s 1000'
work the same way as the conventional 'env export -t -s 1000 1000000'.

Tests cover single flags, grouped flags, the mutex-violation path
(both -bt and -t -b forms) and -s SIZE in joined and separated form.

Adjust CMD_EXPORTENV to select GETOPT so the parser is linked in on
boards that did not already enable it.

Also redo the sep_err logic, use env_set_hex() and shorten error
messages to reduce code size.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v2:
- Redo the sep_err logic to reduce code size

 cmd/Kconfig  |  1 +
 cmd/nvedit.c | 85 ++++++++++++++++++++++------------------------------
 2 files changed, 36 insertions(+), 50 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 8ba7b1e6bd1..b5cc07b499a 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -704,6 +704,7 @@ config CMD_ASKENV
 config CMD_EXPORTENV
 	bool "env export"
 	default y
+	select GETOPT
 	help
 	  Export environments.
 
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 35cc0ee85bf..f2cad21b5f4 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -632,73 +632,63 @@ static int do_env_delete(struct cmd_tbl *cmdtp, int flag,
 static int do_env_export(struct cmd_tbl *cmdtp, int flag,
 			 int argc, char *const argv[])
 {
+	struct getopt_state gs;
 	char	buf[32];
 	ulong	addr;
-	char	*ptr, *cmd, *res;
+	char	*ptr, *res;
 	size_t	size = 0;
 	ssize_t	len;
 	env_t	*envp;
 	char	sep = '\n';
 	int	chk = 0;
 	int	fmt = 0;
+	int	opt;
 
-	cmd = *argv;
-
-	while (--argc > 0 && **++argv == '-') {
-		char *arg = *argv;
-		while (*++arg) {
-			switch (*arg) {
-			case 'b':		/* raw binary format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\0';
-				break;
-			case 'c':		/* external checksum format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\0';
-				chk = 1;
-				break;
-			case 's':		/* size given */
-				if (--argc <= 0)
-					return cmd_usage(cmdtp);
-				size = hextoul(*++argv, NULL);
-				goto NXTARG;
-			case 't':		/* text format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\n';
-				break;
-			default:
-				return CMD_RET_USAGE;
-			}
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, "bcs:t")) > 0) {
+		fmt++;
+		switch (opt) {
+		case 'b':		/* raw binary format */
+			sep = '\0';
+			break;
+		case 'c':		/* external checksum format */
+			sep = '\0';
+			chk = 1;
+			break;
+		case 's':		/* size given */
+			size = hextoul(gs.arg, NULL);
+			fmt--;
+			break;
+		case 't':		/* text format (default) */
+			break;
+		default:
+			return CMD_RET_USAGE;
 		}
-NXTARG:		;
 	}
 
-	if (argc < 1)
+	if (gs.nonopts < 1)
 		return CMD_RET_USAGE;
+	if (fmt > 1) {
+		printf("## Error: %s: only one of -b/c/t allowed\n", argv[0]);
+		return 1;
+	}
 
-	addr = hextoul(argv[0], NULL);
+	addr = hextoul(getopt_pop(&gs), NULL);
 	ptr = map_sysmem(addr, size);
 
 	if (size)
 		memset(ptr, '\0', size);
 
-	argc--;
-	argv++;
-
 	if (sep) {		/* export as text file */
 		len = hexport_r(&env_htab, sep,
-				H_MATCH_KEY | H_MATCH_IDENT,
-				&ptr, size, argc, argv);
+				H_MATCH_KEY | H_MATCH_IDENT, &ptr, size,
+				gs.nonopts, &gs.argv[gs.index]);
 		if (len < 0) {
-			pr_err("## Error: Cannot export environment: errno = %d\n",
+			pr_err("## Error: Cannot export environment (err %dE)\n",
 			       errno);
 			return 1;
 		}
-		sprintf(buf, "%zX", (size_t)len);
-		env_set("filesize", buf);
+		env_set_hex("filesize", len);
 
 		return 0;
 	}
@@ -711,10 +701,10 @@ NXTARG:		;
 		res = ptr;
 
 	len = hexport_r(&env_htab, '\0',
-			H_MATCH_KEY | H_MATCH_IDENT,
-			&res, ENV_SIZE, argc, argv);
+			H_MATCH_KEY | H_MATCH_IDENT, &res, ENV_SIZE,
+			gs.nonopts, &gs.argv[gs.index]);
 	if (len < 0) {
-		pr_err("## Error: Cannot export environment: errno = %d\n",
+		pr_err("## Error: Cannot export environment (err %dE)\n",
 		       errno);
 		return 1;
 	}
@@ -729,11 +719,6 @@ NXTARG:		;
 	env_set_hex("filesize", len + offsetof(env_t, data));
 
 	return 0;
-
-sep_err:
-	printf("## Error: %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",
-	       cmd);
-	return 1;
 }
 #endif
 
-- 
2.43.0



More information about the U-Boot mailing list