These are the gnarliest hand-rolled parsers in nvedit.c Each one 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
env export 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), -s SIZE in joined and separated form, a setenv/export/clear/import roundtrip with -d, and an out-of-order 'import 0x... -t -d' invocation. Adjust CMD_EXPORTENV and CMD_IMPORTENV to select GETOPT so the parser is linked in on boards that did not already enable it. Signed-off-by: Simon Glass <[email protected]> --- cmd/Kconfig | 2 + cmd/nvedit.c | 159 ++++++++++++++++++++++++--------------------------- 2 files changed, 78 insertions(+), 83 deletions(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index 8ba7b1e6bd1..e9bf58b39a6 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -704,12 +704,14 @@ config CMD_ASKENV config CMD_EXPORTENV bool "env export" default y + select GETOPT help Export environments. config CMD_IMPORTENV bool "env import" default y + select GETOPT help Import environments. diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 8561c408992..6af5f52907e 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -632,66 +632,58 @@ 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) { + switch (opt) { + 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 */ + size = hextoul(gs.arg, NULL); + break; + case 't': /* text format */ + if (fmt++) + goto sep_err; + sep = '\n'; + break; + default: + return CMD_RET_USAGE; } -NXTARG: ; } - if (argc < 1) + if (gs.nonopts < 1) 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++; - 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.args[gs.index]); if (len < 0) { pr_err("## Error: Cannot export environment: errno = %d\n", errno); @@ -711,8 +703,8 @@ 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.args[gs.index]); if (len < 0) { pr_err("## Error: Cannot export environment: errno = %d\n", errno); @@ -732,7 +724,7 @@ NXTARG: ; sep_err: printf("## Error: %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n", - cmd); + argv[0]); return 1; } #endif @@ -765,8 +757,9 @@ sep_err: static int do_env_import(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { + struct getopt_state gs; ulong addr; - char *cmd, *ptr; + char *ptr, *tok; char sep = '\n'; int chk = 0; int fmt = 0; @@ -774,55 +767,53 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag, int crlf_is_lf = 0; int wl = 0; size_t size; + 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 't': /* text format */ - if (fmt++) - goto sep_err; - sep = '\n'; - break; - case 'r': /* handle CRLF like LF */ - crlf_is_lf = 1; - break; - case 'd': - del = 1; - break; - default: - return CMD_RET_USAGE; - } + getopt_init_state(&gs, argc, argv); + while ((opt = getopt(&gs, "bctrd")) > 0) { + switch (opt) { + 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 't': /* text format */ + if (fmt++) + goto sep_err; + sep = '\n'; + break; + case 'r': /* handle CRLF like LF */ + crlf_is_lf = 1; + break; + case 'd': + del = 1; + break; + default: + return CMD_RET_USAGE; } } - if (argc < 1) + if (gs.nonopts < 1) return CMD_RET_USAGE; if (!fmt) printf("## Warning: defaulting to text format\n"); - if (sep != '\n' && crlf_is_lf ) + if (sep != '\n' && crlf_is_lf) crlf_is_lf = 0; - addr = hextoul(argv[0], NULL); + addr = hextoul(getopt_pop(&gs), NULL); ptr = map_sysmem(addr, 0); - if (argc >= 2 && strcmp(argv[1], "-")) { - size = hextoul(argv[1], NULL); + tok = getopt_pop(&gs); /* size, "-", or absent */ + if (tok && strcmp(tok, "-")) { + size = hextoul(tok, NULL); } else if (chk) { puts("## Error: external checksum format must pass size\n"); return CMD_RET_FAILURE; @@ -832,7 +823,7 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag, size = 0; while (size < MAX_ENV_SIZE) { - if ((*s == sep) && (*(s+1) == '\0')) + if ((*s == sep) && (*(s + 1) == '\0')) break; ++s; ++size; @@ -845,7 +836,7 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag, printf("## Info: input data size = %zu = 0x%zX\n", size, size); } - if (argc > 2) + if (gs.nonopts > 0) wl = 1; if (chk) { @@ -868,7 +859,9 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag, } if (!himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR, - crlf_is_lf, wl ? argc - 2 : 0, wl ? &argv[2] : NULL)) { + crlf_is_lf, + wl ? gs.nonopts : 0, + wl ? &gs.args[gs.index] : NULL)) { pr_err("## Error: Environment import failed: errno = %d\n", errno); return 1; @@ -879,7 +872,7 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag, sep_err: printf("## %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n", - cmd); + argv[0]); return 1; } #endif -- 2.43.0
