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 <[email protected]> --- (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
