[PATCH v4 10/14] cmd: nvedit: Use getopt() in env export

[Simon Glass]

do_env_export() is one of the gnarliest hand-rolled parsers in the env
code. It walks each - argument by hand, opens an inner while (*++arg)
to split grouped flags, tracks an fmt counter to reject a second
-b/-c/-t and uses a goto NXTARG to skip the rest of the current element
after -s has eaten the following argument for its size.

Replace all of that with getopt(). Move do_env_export() to the
getopt-state command signature, declared with U_BOOT_CMD_MKENT_GETOPT().
The -s argument arrives in gs->arg. Move the format-flag
mutual-exclusion check after the option loop (it is a per-command rule,
not an option-parsing rule) and read the trailing positional list from
gs->argv[gs->index]. Keep the + prefix on the optstring so getopt()
stops at the first non-option, exactly as the hand-rolled loop did.

While here, write the text-format filesize with env_set_hex() rather
than an open-coded sprintf() into a stack buffer. This drops the buffer
and matches the binary path, which already uses env_set_hex(); the only
visible change is that the value is now lower-case hex (its consumers
read it with hextoul(), which is case-insensitive). Check the return
value of both env_set_hex() calls and return 1 on failure, rather than
ignoring it.

Route the env subcommand dispatch in do_env() through cmd_invoke() so
subcommands using either signature are called correctly.

CMD_EXPORTENV selects GETOPT so the parser is linked in on boards that
do 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>
---

(no changes since v2)

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

 cmd/Kconfig  |  1 +
 cmd/nvedit.c | 80 +++++++++++++++++++++++++---------------------------
 2 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 3c5abddd31f..e3d92055f17 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 636bddee1be..c74898a05fb 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -29,6 +29,7 @@
 #include <console.h>
 #include <env.h>
 #include <env_internal.h>
+#include <getopt.h>
 #include <log.h>
 #include <search.h>
 #include <errno.h>
@@ -631,10 +632,8 @@ static int do_env_delete(struct cmd_tbl *cmdtp, int flag,
  *
  *	=> env import -d -t ${backup_addr}
  */
-static int do_env_export(struct cmd_tbl *cmdtp, int flag,
-			 int argc, char *const argv[])
+static int do_env_export(struct getopt_state *gs)
 {
-	char	buf[32];
 	ulong	addr;
 	char	*ptr, *cmd, *res;
 	size_t	size = 0;
@@ -643,52 +642,48 @@ static int do_env_export(struct cmd_tbl *cmdtp, int flag,
 	char	sep = '\n';
 	int	chk = 0;
 	int	fmt = 0;
+	int	argc;
+	char	*const *argv;
+	int	opt;
 
-	cmd = *argv;
+	cmd = gs->argv[0];
 
-	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;
-			}
+	while ((opt = getopt(gs, "+bcs:t")) > 0) {
+		switch (opt) {
+		case 'b':		/* raw binary format */
+			sep = '\0';
+			fmt++;
+			break;
+		case 'c':		/* external checksum format */
+			sep = '\0';
+			chk = 1;
+			fmt++;
+			break;
+		case 's':		/* size given */
+			size = hextoul(gs->arg, NULL);
+			break;
+		case 't':		/* text format */
+			sep = '\n';
+			fmt++;
+			break;
+		default:
+			return CMD_RET_USAGE;
 		}
-NXTARG:		;
 	}
+	if (fmt > 1)
+		goto sep_err;
 
-	if (argc < 1)
+	if (gs->index >= gs->argc)
 		return CMD_RET_USAGE;
 
-	addr = hextoul(argv[0], NULL);
+	addr = hextoul(getopt_pop(gs), NULL);
 	ptr = map_sysmem(addr, size);
 
 	if (size)
 		memset(ptr, '\0', size);
 
-	argc--;
-	argv++;
+	argc = gs->argc - gs->index;
+	argv = &gs->argv[gs->index];
 
 	if (sep) {		/* export as text file */
 		len = hexport_r(&env_htab, sep,
@@ -699,8 +694,8 @@ NXTARG:		;
 			       errno);
 			return 1;
 		}
-		sprintf(buf, "%zX", (size_t)len);
-		env_set("filesize", buf);
+		if (env_set_hex("filesize", len))
+			return 1;
 
 		return 0;
 	}
@@ -728,7 +723,8 @@ NXTARG:		;
 		envp->flags = ENV_REDUND_ACTIVE;
 #endif
 	}
-	env_set_hex("filesize", len + offsetof(env_t, data));
+	if (env_set_hex("filesize", len + offsetof(env_t, data)))
+		return 1;
 
 	return 0;
 
@@ -1077,7 +1073,7 @@ static struct cmd_tbl cmd_env_sub[] = {
 	U_BOOT_CMD_MKENT(flags, 1, 0, do_env_flags, "", ""),
 #endif
 #if defined(CONFIG_CMD_EXPORTENV)
-	U_BOOT_CMD_MKENT(export, 4, 0, do_env_export, "", ""),
+	U_BOOT_CMD_MKENT_GETOPT(export, 4, 0, do_env_export, "", ""),
 #endif
 #if defined(CONFIG_CMD_GREPENV)
 	U_BOOT_CMD_MKENT(grep, CONFIG_SYS_MAXARGS, 1, do_env_grep, "", ""),
@@ -1127,7 +1123,7 @@ static int do_env(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	cp = find_cmd_tbl(argv[0], cmd_env_sub, ARRAY_SIZE(cmd_env_sub));
 
 	if (cp)
-		return cp->cmd(cmdtp, flag, argc, argv);
+		return cmd_invoke(cp, flag, argc, argv);
 
 	return CMD_RET_USAGE;
 }
-- 
2.43.0