[U-Boot] [PATCH v3 0/7] tools: env: simplify argument parsing
Currently parameter parsing happens in two steps. First with getopt at the beginning of main to parse the common options. Then second is using adhoc parsing in printenv/setenv code path. When doing adhoc parsing, 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.
v3: - rebased on top of master/8555dd8 - made emails in From:/Signed-off header match
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 | 24 +++++ tools/env/fw_env_main.c | 249 ++++++++++++++++++++++++++++++++++-------------- 5 files changed, 239 insertions(+), 177 deletions(-)
passing argv/argc can produce off-by-one errors
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- 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)
On Wed, Dec 09, 2015 at 01:13:21PM +0100, Andreas Fenkart wrote:
passing argv/argc can produce off-by-one errors
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com
Applied to u-boot/master, thanks!
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- 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; }
On Wed, Dec 09, 2015 at 01:13:22PM +0100, Andreas Fenkart wrote:
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com
Applied to u-boot/master, thanks!
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@digitalstrom.com --- 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;
On Wed, Dec 09, 2015 at 01:13:23PM +0100, Andreas Fenkart wrote:
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@digitalstrom.com
Applied to u-boot/master, thanks!
disabled original parsing, but not yet removed since the argument indexing needs to be fixed
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- tools/env/fw_env.c | 64 ++++++++++--------------------------------------- tools/env/fw_env.h | 15 ++++++++++++ tools/env/fw_env_main.c | 31 ++++++++++++++++++++---- 3 files changed, 53 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..57149e7 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,17 @@ "bootm" #endif
+struct common_args { +#ifdef CONFIG_FILE + char *config_file; +#endif + 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 +76,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;
On Wed, Dec 09, 2015 at 01:13:24PM +0100, Andreas Fenkart wrote:
disabled original parsing, but not yet removed since the argument indexing needs to be fixed
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com
Applied to u-boot/master, thanks!
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- 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);
On Wed, Dec 09, 2015 at 01:13:25PM +0100, Andreas Fenkart wrote:
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com
Applied to u-boot/master, thanks!
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- tools/env/fw_env_main.c | 62 +++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c index daf4688..39c8771 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,13 +89,13 @@ 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; @@ -104,12 +104,37 @@ int parse_printenv_args(int argc, char *argv[]) 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 +149,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();
On Wed, Dec 09, 2015 at 01:13:26PM +0100, Andreas Fenkart wrote:
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com
Applied to u-boot/master, thanks!
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com --- 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 39c8771..4bd4216 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[]) @@ -105,7 +131,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: @@ -137,7 +163,7 @@ int parse_printenv_args(int argc, char *argv[]) /* ignore common options */ break; default: /* '?' */ - usage(); + usage_printenv(); exit(EXIT_FAILURE); break; } @@ -163,7 +189,7 @@ int parse_setenv_args(int argc, char *argv[]) /* ignore common options */ break; default: /* '?' */ - usage(); + usage_setenv(); exit(EXIT_FAILURE); break; } @@ -173,27 +199,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; @@ -210,10 +243,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;
On Wed, Dec 09, 2015 at 01:13:27PM +0100, Andreas Fenkart wrote:
Signed-off-by: Andreas Fenkart andreas.fenkart@digitalstrom.com
Applied to u-boot/master, thanks!
participants (2)
-
Andreas Fenkart -
Tom Rini