Nico Huber submitted this change.
cli_classic: Tidy up some repeated handling patterns into funcs
Introduce cli_classic_single_operation() to consolidate the repeating
pattern of multiple CLI operations at once. Also modify
cli_classic_abort_usage() to take an optional error abort string and
print it to stderr, this allows for trimming a few more lines off the
cli implementation.
V.2: A few fixes upon review:
- Trim off some unnecessary braces for single line branches.
- Pass 'operation_specified' by reference.
- Rename a function.
V.3: Fix print order of cli_classic_abort_usage().
Change-Id: I54598efdaee2b95cb278b0f2aac05f48bbd95bef
Signed-off-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/35611
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
---
M cli_classic.c
1 file changed, 64 insertions(+), 131 deletions(-)
diff --git a/cli_classic.c b/cli_classic.c
index de6ce04..73cc417 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -76,12 +76,21 @@
"If no operation is specified, flashrom will only probe for flash chips.\n");
}
-static void cli_classic_abort_usage(void)
+static void cli_classic_abort_usage(const char *msg)
{
+ if (msg)
+ fprintf(stderr, "%s", msg);
printf("Please run \"flashrom --help\" for usage info.\n");
exit(1);
}
+static void cli_classic_validate_singleop(int *operation_specified)
+{
+ if (++(*operation_specified) > 1) {
+ cli_classic_abort_usage("More than one operation specified. Aborting.\n");
+ }
+}
+
static int check_filename(char *filename, const char *type)
{
if (!filename || (filename[0] == '\0')) {
@@ -177,41 +186,27 @@
long_options, &option_index)) != EOF) {
switch (opt) {
case 'r':
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
filename = strdup(optarg);
read_it = 1;
break;
case 'w':
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
filename = strdup(optarg);
write_it = 1;
break;
case 'v':
//FIXME: gracefully handle superfluous -v
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
if (dont_verify_it) {
- fprintf(stderr, "--verify and --noverify are mutually exclusive. Aborting.\n");
- cli_classic_abort_usage();
+ cli_classic_abort_usage("--verify and --noverify are mutually exclusive. Aborting.\n");
}
filename = strdup(optarg);
verify_it = 1;
break;
case 'n':
if (verify_it) {
- fprintf(stderr, "--verify and --noverify are mutually exclusive. Aborting.\n");
- cli_classic_abort_usage();
+ cli_classic_abort_usage("--verify and --noverify are mutually exclusive. Aborting.\n");
}
dont_verify_it = 1;
break;
@@ -227,132 +222,87 @@
verbose_logfile = verbose_screen;
break;
case 'E':
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
erase_it = 1;
break;
case 'f':
force = 1;
break;
case 'l':
- if (layoutfile) {
- fprintf(stderr, "Error: --layout specified "
- "more than once. Aborting.\n");
- cli_classic_abort_usage();
- }
- if (ifd) {
- fprintf(stderr, "Error: --layout and --ifd both specified. Aborting.\n");
- cli_classic_abort_usage();
- }
- if (fmap) {
- fprintf(stderr, "Error: --layout and --fmap-file both specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ if (layoutfile)
+ cli_classic_abort_usage("Error: --layout specified more than once. Aborting.\n");
+ if (ifd)
+ cli_classic_abort_usage("Error: --layout and --ifd both specified. Aborting.\n");
+ if (fmap)
+ cli_classic_abort_usage("Error: --layout and --fmap-file both specified. Aborting.\n");
layoutfile = strdup(optarg);
break;
case OPTION_IFD:
- if (layoutfile) {
- fprintf(stderr, "Error: --layout and --ifd both specified. Aborting.\n");
- cli_classic_abort_usage();
- }
- if (fmap) {
- fprintf(stderr, "Error: --fmap-file and --ifd both specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ if (layoutfile)
+ cli_classic_abort_usage("Error: --layout and --ifd both specified. Aborting.\n");
+ if (fmap)
+ cli_classic_abort_usage("Error: --fmap-file and --ifd both specified. Aborting.\n");
ifd = 1;
break;
case OPTION_FMAP_FILE:
- if (fmap) {
- fprintf(stderr, "Error: --fmap or --fmap-file specified "
+ if (fmap)
+ cli_classic_abort_usage("Error: --fmap or --fmap-file specified "
"more than once. Aborting.\n");
- cli_classic_abort_usage();
- }
- if (ifd) {
- fprintf(stderr, "Error: --fmap-file and --ifd both specified. Aborting.\n");
- cli_classic_abort_usage();
- }
- if (layoutfile) {
- fprintf(stderr, "Error: --fmap-file and --layout both specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ if (ifd)
+ cli_classic_abort_usage("Error: --fmap-file and --ifd both specified. Aborting.\n");
+ if (layoutfile)
+ cli_classic_abort_usage("Error: --fmap-file and --layout both specified. Aborting.\n");
fmapfile = strdup(optarg);
fmap = 1;
break;
case OPTION_FMAP:
- if (fmap) {
- fprintf(stderr, "Error: --fmap or --fmap-file specified "
+ if (fmap)
+ cli_classic_abort_usage("Error: --fmap or --fmap-file specified "
"more than once. Aborting.\n");
- cli_classic_abort_usage();
- }
- if (ifd) {
- fprintf(stderr, "Error: --fmap and --ifd both specified. Aborting.\n");
- cli_classic_abort_usage();
- }
- if (layoutfile) {
- fprintf(stderr, "Error: --layout and --fmap both specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ if (ifd)
+ cli_classic_abort_usage("Error: --fmap and --ifd both specified. Aborting.\n");
+ if (layoutfile)
+ cli_classic_abort_usage("Error: --layout and --fmap both specified. Aborting.\n");
fmap = 1;
break;
case 'i':
tempstr = strdup(optarg);
if (register_include_arg(&include_args, tempstr)) {
free(tempstr);
- cli_classic_abort_usage();
+ cli_classic_abort_usage(NULL);
}
break;
case OPTION_FLASH_CONTENTS:
referencefile = strdup(optarg);
break;
case OPTION_FLASH_NAME:
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
flash_name = 1;
break;
case OPTION_FLASH_SIZE:
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
flash_size = 1;
break;
case 'L':
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
list_supported = 1;
break;
case 'z':
#if CONFIG_PRINT_WIKI == 1
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
list_supported_wiki = 1;
#else
- fprintf(stderr, "Error: Wiki output was not compiled "
- "in. Aborting.\n");
- cli_classic_abort_usage();
+ cli_classic_abort_usage("Error: Wiki output was not"
+ "compiled in. Aborting.\n");
#endif
break;
case 'p':
if (prog != PROGRAMMER_INVALID) {
- fprintf(stderr, "Error: --programmer specified "
+ cli_classic_abort_usage("Error: --programmer specified "
"more than once. You can separate "
"multiple\nparameters for a programmer "
"with \",\". Please see the man page "
"for details.\n");
- cli_classic_abort_usage();
}
for (prog = 0; prog < PROGRAMMER_INVALID; prog++) {
name = programmer_table[prog].name;
@@ -384,31 +334,22 @@
optarg);
list_programmers_linebreak(0, 80, 0);
msg_ginfo(".\n");
- cli_classic_abort_usage();
+ cli_classic_abort_usage(NULL);
}
break;
case 'R':
/* print_version() is always called during startup. */
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
exit(0);
break;
case 'h':
- if (++operation_specified > 1) {
- fprintf(stderr, "More than one operation "
- "specified. Aborting.\n");
- cli_classic_abort_usage();
- }
+ cli_classic_validate_singleop(&operation_specified);
cli_classic_usage(argv[0]);
exit(0);
break;
case 'o':
#ifdef STANDALONE
- fprintf(stderr, "Log file not supported in standalone mode. Aborting.\n");
- cli_classic_abort_usage();
+ cli_classic_abort_usage("Log file not supported in standalone mode. Aborting.\n");
#else /* STANDALONE */
if (logfile) {
fprintf(stderr, "Warning: -o/--output specified multiple times.\n");
@@ -417,40 +358,32 @@
logfile = strdup(optarg);
if (logfile[0] == '\0') {
- fprintf(stderr, "No log filename specified.\n");
- cli_classic_abort_usage();
+ cli_classic_abort_usage("No log filename specified.\n");
}
#endif /* STANDALONE */
break;
default:
- cli_classic_abort_usage();
+ cli_classic_abort_usage(NULL);
break;
}
}
- if (optind < argc) {
- fprintf(stderr, "Error: Extra parameter found.\n");
- cli_classic_abort_usage();
- }
-
- if ((read_it | write_it | verify_it) && check_filename(filename, "image")) {
- cli_classic_abort_usage();
- }
- if (layoutfile && check_filename(layoutfile, "layout")) {
- cli_classic_abort_usage();
- }
- if (fmapfile && check_filename(fmapfile, "fmap")) {
- cli_classic_abort_usage();
- }
- if (referencefile && check_filename(referencefile, "reference")) {
- cli_classic_abort_usage();
- }
+ if (optind < argc)
+ cli_classic_abort_usage("Error: Extra parameter found.\n");
+ if ((read_it | write_it | verify_it) && check_filename(filename, "image"))
+ cli_classic_abort_usage(NULL);
+ if (layoutfile && check_filename(layoutfile, "layout"))
+ cli_classic_abort_usage(NULL);
+ if (fmapfile && check_filename(fmapfile, "fmap"))
+ cli_classic_abort_usage(NULL);
+ if (referencefile && check_filename(referencefile, "reference"))
+ cli_classic_abort_usage(NULL);
#ifndef STANDALONE
if (logfile && check_filename(logfile, "log"))
- cli_classic_abort_usage();
+ cli_classic_abort_usage(NULL);
if (logfile && open_logfile(logfile))
- cli_classic_abort_usage();
+ cli_classic_abort_usage(NULL);
#endif /* !STANDALONE */
#if CONFIG_PRINT_WIKI == 1
To view, visit change 35611. To unsubscribe, or for help writing mail filters, visit settings.