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>

1 files changed, 64 insertions, 131 deletions

diff --git a/cli_classic.c b/cli_classic.c
index de6ce04b7..73cc41791 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -76,12 +76,21 @@ static void cli_classic_usage(const char *name)
 	       "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 @@ int main(int argc, char *argv[])
 				  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 @@ int main(int argc, char *argv[])
 				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 @@ int main(int argc, char *argv[])
 					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 @@ int main(int argc, char *argv[])
 
 			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