Anastasia Klimchuk submitted this change.

View Change

cli_classic: refactor arguments parsing into separate func

Move variables that represent parsed options to `cli_options`
structure. This patchset also introduces the following functions:

- parse_options() which parses command line arguments and fills
the structure.
- free_options() that releases an allocated data from the
structure.

This is one of the steps on the way to simplify the main function
by using helper functions with descriptive names.

TOPIC=split_main_func
TEST=the following scenarious run successfully

./flashrom -p dummy:emulate=S25FL128L -V
./flashrom -p dummy:size=8388608,emulate=VARIABLE_SIZE \
    -r /tmp/dump.rom
./flashrom -p dummy:emulate=W25Q128FV -l /tmp/rom.layout \
    -i boot -w /tmp/rom.tr.img
./flashrom -p dummy:emulate=W25Q128FV --wp-list
./flashrom -p dummy:emulate=W25Q128FV,hwwp=yes \
    --wp-enable \
    --wp-range=0x00c00000,0x00400000 \
    --wp-status
$ head -c 16MiB </dev/urandom >/tmp/image.rom
./flashrom -p dummy:image=/tmp/image.rom,emulate=S25FL128L \
    -c S25FL128L -E -V

Change-Id: Id573bc74e3bb46b7dc42f1452fff6394d4f091bc
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66343
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
---
M cli_classic.c
1 file changed, 409 insertions(+), 343 deletions(-)

diff --git a/cli_classic.c b/cli_classic.c
index da68919..787de67 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -31,6 +31,53 @@
 #include "programmer.h"
 #include "libflashrom.h"
 
+enum {
+	OPTION_IFD = 0x0100,
+	OPTION_FMAP,
+	OPTION_FMAP_FILE,
+	OPTION_FLASH_CONTENTS,
+	OPTION_FLASH_NAME,
+	OPTION_FLASH_SIZE,
+	OPTION_WP_STATUS,
+	OPTION_WP_SET_RANGE,
+	OPTION_WP_SET_REGION,
+	OPTION_WP_ENABLE,
+	OPTION_WP_DISABLE,
+	OPTION_WP_LIST,
+	OPTION_PROGRESS,
+};
+
+struct cli_options {
+	bool read_it, extract_it, write_it, erase_it, verify_it;
+	bool dont_verify_it, dont_verify_all;
+	bool list_supported;
+#if CONFIG_PRINT_WIKI == 1
+	bool list_supported_wiki;
+#endif
+	char *filename;
+
+	const struct programmer_entry *prog;
+	char *pparam;
+
+	bool ifd, fmap;
+	struct flashrom_layout *layout;
+	struct layout_include_args *include_args;
+	char *layoutfile;
+	char *fmapfile;
+
+	unsigned int wp_start, wp_len;
+	bool enable_wp, disable_wp, print_wp_status;
+	bool set_wp_range, set_wp_region, print_wp_ranges;
+	char *wp_region;
+
+	bool force;
+	bool flash_name, flash_size;
+	bool show_progress;
+	char *logfile;
+	char *referencefile;
+	const char *chip_to_probe;
+};
+
 static void cli_classic_usage(const char *name)
 {
 	printf("Usage: %s [-h|-R|-L|"
@@ -565,47 +612,260 @@
 	return limitexceeded;
 }
 
+static void parse_options(int argc, char **argv, const char *optstring,
+			  const struct option *long_options,
+			  struct cli_options *options)
+{
+	const char *name;
+	int namelen, opt;
+	int option_index = 0, operation_specified = 0;
+
+	/* FIXME: Delay all operation_specified checks until after command
+	 * line parsing to allow --help overriding everything else.
+	 */
+	while ((opt = getopt_long(argc, argv, optstring,
+				  long_options, &option_index)) != EOF) {
+		switch (opt) {
+		case 'r':
+			cli_classic_validate_singleop(&operation_specified);
+			options->filename = strdup(optarg);
+			options->read_it = true;
+			break;
+		case 'w':
+			cli_classic_validate_singleop(&operation_specified);
+			options->filename = strdup(optarg);
+			options->write_it = true;
+			break;
+		case 'v':
+			//FIXME: gracefully handle superfluous -v
+			cli_classic_validate_singleop(&operation_specified);
+			if (options->dont_verify_it) {
+				cli_classic_abort_usage("--verify and --noverify are mutually exclusive. Aborting.\n");
+			}
+			options->filename = strdup(optarg);
+			options->verify_it = true;
+			break;
+		case 'n':
+			if (options->verify_it) {
+				cli_classic_abort_usage("--verify and --noverify are mutually exclusive. Aborting.\n");
+			}
+			options->dont_verify_it = true;
+			break;
+		case 'N':
+			options->dont_verify_all = true;
+			break;
+		case 'x':
+			cli_classic_validate_singleop(&operation_specified);
+			options->extract_it = true;
+			break;
+		case 'c':
+			options->chip_to_probe = strdup(optarg);
+			break;
+		case 'V':
+			verbose_screen++;
+			if (verbose_screen > FLASHROM_MSG_DEBUG2)
+				verbose_logfile = verbose_screen;
+			break;
+		case 'E':
+			cli_classic_validate_singleop(&operation_specified);
+			options->erase_it = true;
+			break;
+		case 'f':
+			options->force = true;
+			break;
+		case 'l':
+			if (options->layoutfile)
+				cli_classic_abort_usage("Error: --layout specified more than once. Aborting.\n");
+			if (options->ifd)
+				cli_classic_abort_usage("Error: --layout and --ifd both specified. Aborting.\n");
+			if (options->fmap)
+				cli_classic_abort_usage("Error: --layout and --fmap-file both specified. Aborting.\n");
+			options->layoutfile = strdup(optarg);
+			break;
+		case OPTION_IFD:
+			if (options->layoutfile)
+				cli_classic_abort_usage("Error: --layout and --ifd both specified. Aborting.\n");
+			if (options->fmap)
+				cli_classic_abort_usage("Error: --fmap-file and --ifd both specified. Aborting.\n");
+			options->ifd = true;
+			break;
+		case OPTION_FMAP_FILE:
+			if (options->fmap)
+				cli_classic_abort_usage("Error: --fmap or --fmap-file specified "
+					"more than once. Aborting.\n");
+			if (options->ifd)
+				cli_classic_abort_usage("Error: --fmap-file and --ifd both specified. Aborting.\n");
+			if (options->layoutfile)
+				cli_classic_abort_usage("Error: --fmap-file and --layout both specified. Aborting.\n");
+			options->fmapfile = strdup(optarg);
+			options->fmap = true;
+			break;
+		case OPTION_FMAP:
+			if (options->fmap)
+				cli_classic_abort_usage("Error: --fmap or --fmap-file specified "
+					"more than once. Aborting.\n");
+			if (options->ifd)
+				cli_classic_abort_usage("Error: --fmap and --ifd both specified. Aborting.\n");
+			if (options->layoutfile)
+				cli_classic_abort_usage("Error: --layout and --fmap both specified. Aborting.\n");
+			options->fmap = true;
+			break;
+		case 'i':
+			if (register_include_arg(&options->include_args, optarg))
+				cli_classic_abort_usage(NULL);
+			break;
+		case OPTION_FLASH_CONTENTS:
+			if (options->referencefile)
+				cli_classic_abort_usage("Error: --flash-contents specified more than once."
+							"Aborting.\n");
+			options->referencefile = strdup(optarg);
+			break;
+		case OPTION_FLASH_NAME:
+			cli_classic_validate_singleop(&operation_specified);
+			options->flash_name = true;
+			break;
+		case OPTION_FLASH_SIZE:
+			cli_classic_validate_singleop(&operation_specified);
+			options->flash_size = true;
+			break;
+		case OPTION_WP_STATUS:
+			options->print_wp_status = true;
+			break;
+		case OPTION_WP_LIST:
+			options->print_wp_ranges = true;
+			break;
+		case OPTION_WP_SET_RANGE:
+			if (parse_wp_range(&options->wp_start, &options->wp_len) < 0)
+				cli_classic_abort_usage("Incorrect wp-range arguments provided.\n");
+
+			options->set_wp_range = true;
+			break;
+		case OPTION_WP_SET_REGION:
+			options->set_wp_region = true;
+			options->wp_region = strdup(optarg);
+			break;
+		case OPTION_WP_ENABLE:
+			options->enable_wp = true;
+			break;
+		case OPTION_WP_DISABLE:
+			options->disable_wp = true;
+			break;
+		case 'L':
+			cli_classic_validate_singleop(&operation_specified);
+			options->list_supported = true;
+			break;
+		case 'z':
+#if CONFIG_PRINT_WIKI == 1
+			cli_classic_validate_singleop(&operation_specified);
+			options->list_supported_wiki = true;
+#else
+			cli_classic_abort_usage("Error: Wiki output was not "
+					"compiled in. Aborting.\n");
+#endif
+			break;
+		case 'p':
+			if (options->prog != NULL) {
+				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");
+			}
+			size_t p;
+			for (p = 0; p < programmer_table_size; p++) {
+				name = programmer_table[p]->name;
+				namelen = strlen(name);
+				if (strncmp(optarg, name, namelen) == 0) {
+					switch (optarg[namelen]) {
+					case ':':
+						options->pparam = strdup(optarg + namelen + 1);
+						if (!strlen(options->pparam)) {
+							free(options->pparam);
+							options->pparam = NULL;
+						}
+						options->prog = programmer_table[p];
+						break;
+					case '\0':
+						options->prog = programmer_table[p];
+						break;
+					default:
+						/* The continue refers to the
+						 * for loop. It is here to be
+						 * able to differentiate between
+						 * foo and foobar.
+						 */
+						continue;
+					}
+					break;
+				}
+			}
+			if (options->prog == NULL) {
+				fprintf(stderr, "Error: Unknown programmer \"%s\". Valid choices are:\n",
+					optarg);
+				list_programmers_linebreak(0, 80, 0);
+				msg_ginfo(".\n");
+				cli_classic_abort_usage(NULL);
+			}
+			break;
+		case 'R':
+			/* print_version() is always called during startup. */
+			cli_classic_validate_singleop(&operation_specified);
+			exit(0);
+			break;
+		case 'h':
+			cli_classic_validate_singleop(&operation_specified);
+			cli_classic_usage(argv[0]);
+			exit(0);
+			break;
+		case 'o':
+			if (options->logfile) {
+				fprintf(stderr, "Warning: -o/--output specified multiple times.\n");
+				free(options->logfile);
+			}
+
+			options->logfile = strdup(optarg);
+			if (options->logfile[0] == '\0') {
+				cli_classic_abort_usage("No log filename specified.\n");
+			}
+			break;
+		case OPTION_PROGRESS:
+			options->show_progress = true;
+			break;
+		default:
+			cli_classic_abort_usage(NULL);
+			break;
+		}
+	}
+
+	if (optind < argc)
+		cli_classic_abort_usage("Error: Extra parameter found.\n");
+}
+
+static void free_options(struct cli_options *options)
+{
+	cleanup_include_args(&options->include_args);
+	free(options->filename);
+	free(options->fmapfile);
+	free(options->referencefile);
+	free(options->layoutfile);
+	free(options->pparam);
+	free(options->wp_region);
+	free(options->logfile);
+	free((char *)options->chip_to_probe);
+}
+
 int main(int argc, char *argv[])
 {
 	const struct flashchip *chip = NULL;
 	/* Probe for up to eight flash chips. */
 	struct flashctx flashes[8] = {{0}};
 	struct flashctx *fill_flash;
-	const char *name;
-	int namelen, opt, i, j;
-	int startchip = -1, chipcount = 0, option_index = 0;
-	int operation_specified = 0;
-	unsigned int wp_start = 0, wp_len = 0;
-	bool force = false, ifd = false, fmap = false;
-#if CONFIG_PRINT_WIKI == 1
-	bool list_supported_wiki = false;
-#endif
-	bool flash_name = false, flash_size = false;
-	bool enable_wp = false, disable_wp = false, print_wp_status = false;
-	bool set_wp_range = false, set_wp_region = false, print_wp_ranges = false;
-	bool read_it = false, extract_it = false, write_it = false, erase_it = false, verify_it = false;
-	bool dont_verify_it = false, dont_verify_all = false;
-	bool list_supported = false;
-	bool show_progress = false;
-	struct flashrom_layout *layout = NULL;
-	static const struct programmer_entry *prog = NULL;
-	enum {
-		OPTION_IFD = 0x0100,
-		OPTION_FMAP,
-		OPTION_FMAP_FILE,
-		OPTION_FLASH_CONTENTS,
-		OPTION_FLASH_NAME,
-		OPTION_FLASH_SIZE,
-		OPTION_WP_STATUS,
-		OPTION_WP_SET_RANGE,
-		OPTION_WP_SET_REGION,
-		OPTION_WP_ENABLE,
-		OPTION_WP_DISABLE,
-		OPTION_WP_LIST,
-		OPTION_PROGRESS,
-	};
+	char *tempstr = NULL;
+	int startchip = -1, chipcount = 0;
+	int i, j;
 	int ret = 0;
 
+	struct cli_options options = { 0 };
 	static const char optstring[] = "r:Rw:v:nNVEfc:l:i:p:Lzho:x";
 	static const struct option long_options[] = {
 		{"read",		1, NULL, 'r'},
@@ -644,17 +904,6 @@
 		{NULL,			0, NULL, 0},
 	};
 
-	char *filename = NULL;
-	char *referencefile = NULL;
-	char *layoutfile = NULL;
-	char *fmapfile = NULL;
-	char *logfile = NULL;
-	char *tempstr = NULL;
-	char *pparam = NULL;
-	struct layout_include_args *include_args = NULL;
-	char *wp_region = NULL;
-	const char *chip_to_probe = NULL;
-
 	/*
 	 * Safety-guard against a user who has (mistakenly) closed
 	 * stdout or stderr before exec'ing flashrom.  We disable
@@ -673,246 +922,30 @@
 		exit(1);
 
 	setbuf(stdout, NULL);
-	/* FIXME: Delay all operation_specified checks until after command
-	 * line parsing to allow --help overriding everything else.
-	 */
-	while ((opt = getopt_long(argc, argv, optstring,
-				  long_options, &option_index)) != EOF) {
-		switch (opt) {
-		case 'r':
-			cli_classic_validate_singleop(&operation_specified);
-			filename = strdup(optarg);
-			read_it = true;
-			break;
-		case 'w':
-			cli_classic_validate_singleop(&operation_specified);
-			filename = strdup(optarg);
-			write_it = true;
-			break;
-		case 'v':
-			//FIXME: gracefully handle superfluous -v
-			cli_classic_validate_singleop(&operation_specified);
-			if (dont_verify_it) {
-				cli_classic_abort_usage("--verify and --noverify are mutually exclusive. Aborting.\n");
-			}
-			filename = strdup(optarg);
-			verify_it = true;
-			break;
-		case 'n':
-			if (verify_it) {
-				cli_classic_abort_usage("--verify and --noverify are mutually exclusive. Aborting.\n");
-			}
-			dont_verify_it = true;
-			break;
-		case 'N':
-			dont_verify_all = true;
-			break;
-		case 'x':
-			cli_classic_validate_singleop(&operation_specified);
-			extract_it = true;
-			break;
-		case 'c':
-			chip_to_probe = strdup(optarg);
-			break;
-		case 'V':
-			verbose_screen++;
-			if (verbose_screen > FLASHROM_MSG_DEBUG2)
-				verbose_logfile = verbose_screen;
-			break;
-		case 'E':
-			cli_classic_validate_singleop(&operation_specified);
-			erase_it = true;
-			break;
-		case 'f':
-			force = true;
-			break;
-		case 'l':
-			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)
-				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 = true;
-			break;
-		case OPTION_FMAP_FILE:
-			if (fmap)
-				cli_classic_abort_usage("Error: --fmap or --fmap-file specified "
-					"more than once. Aborting.\n");
-			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 = true;
-			break;
-		case OPTION_FMAP:
-			if (fmap)
-				cli_classic_abort_usage("Error: --fmap or --fmap-file specified "
-					"more than once. Aborting.\n");
-			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 = true;
-			break;
-		case 'i':
-			if (register_include_arg(&include_args, optarg))
-				cli_classic_abort_usage(NULL);
-			break;
-		case OPTION_FLASH_CONTENTS:
-			if (referencefile)
-				cli_classic_abort_usage("Error: --flash-contents specified more than once."
-							"Aborting.\n");
-			referencefile = strdup(optarg);
-			break;
-		case OPTION_FLASH_NAME:
-			cli_classic_validate_singleop(&operation_specified);
-			flash_name = true;
-			break;
-		case OPTION_FLASH_SIZE:
-			cli_classic_validate_singleop(&operation_specified);
-			flash_size = true;
-			break;
-		case OPTION_WP_STATUS:
-			print_wp_status = true;
-			break;
-		case OPTION_WP_LIST:
-			print_wp_ranges = true;
-			break;
-		case OPTION_WP_SET_RANGE:
-			if (parse_wp_range(&wp_start, &wp_len) < 0)
-				cli_classic_abort_usage("Incorrect wp-range arguments provided.\n");
 
-			set_wp_range = true;
-			break;
-		case OPTION_WP_SET_REGION:
-			set_wp_region = true;
-			wp_region = strdup(optarg);
-			break;
-		case OPTION_WP_ENABLE:
-			enable_wp = true;
-			break;
-		case OPTION_WP_DISABLE:
-			disable_wp = true;
-			break;
-		case 'L':
-			cli_classic_validate_singleop(&operation_specified);
-			list_supported = true;
-			break;
-		case 'z':
-#if CONFIG_PRINT_WIKI == 1
-			cli_classic_validate_singleop(&operation_specified);
-			list_supported_wiki = true;
-#else
-			cli_classic_abort_usage("Error: Wiki output was not "
-					"compiled in. Aborting.\n");
-#endif
-			break;
-		case 'p':
-			if (prog != NULL) {
-				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");
-			}
-			size_t p;
-			for (p = 0; p < programmer_table_size; p++) {
-				name = programmer_table[p]->name;
-				namelen = strlen(name);
-				if (strncmp(optarg, name, namelen) == 0) {
-					switch (optarg[namelen]) {
-					case ':':
-						pparam = strdup(optarg + namelen + 1);
-						if (!strlen(pparam)) {
-							free(pparam);
-							pparam = NULL;
-						}
-						prog = programmer_table[p];
-						break;
-					case '\0':
-						prog = programmer_table[p];
-						break;
-					default:
-						/* The continue refers to the
-						 * for loop. It is here to be
-						 * able to differentiate between
-						 * foo and foobar.
-						 */
-						continue;
-					}
-					break;
-				}
-			}
-			if (prog == NULL) {
-				fprintf(stderr, "Error: Unknown programmer \"%s\". Valid choices are:\n",
-					optarg);
-				list_programmers_linebreak(0, 80, 0);
-				msg_ginfo(".\n");
-				cli_classic_abort_usage(NULL);
-			}
-			break;
-		case 'R':
-			/* print_version() is always called during startup. */
-			cli_classic_validate_singleop(&operation_specified);
-			exit(0);
-			break;
-		case 'h':
-			cli_classic_validate_singleop(&operation_specified);
-			cli_classic_usage(argv[0]);
-			exit(0);
-			break;
-		case 'o':
-			if (logfile) {
-				fprintf(stderr, "Warning: -o/--output specified multiple times.\n");
-				free(logfile);
-			}
+	parse_options(argc, argv, optstring, long_options, &options);
 
-			logfile = strdup(optarg);
-			if (logfile[0] == '\0') {
-				cli_classic_abort_usage("No log filename specified.\n");
-			}
-			break;
-		case OPTION_PROGRESS:
-			show_progress = true;
-			break;
-		default:
-			cli_classic_abort_usage(NULL);
-			break;
-		}
-	}
-
-	if (optind < argc)
-		cli_classic_abort_usage("Error: Extra parameter found.\n");
-	if ((read_it | write_it | verify_it) && check_filename(filename, "image"))
+	if ((options.read_it | options.write_it | options.verify_it) && check_filename(options.filename, "image"))
 		cli_classic_abort_usage(NULL);
-	if (layoutfile && check_filename(layoutfile, "layout"))
+	if (options.layoutfile && check_filename(options.layoutfile, "layout"))
 		cli_classic_abort_usage(NULL);
-	if (fmapfile && check_filename(fmapfile, "fmap"))
+	if (options.fmapfile && check_filename(options.fmapfile, "fmap"))
 		cli_classic_abort_usage(NULL);
-	if (referencefile && check_filename(referencefile, "reference"))
+	if (options.referencefile && check_filename(options.referencefile, "reference"))
 		cli_classic_abort_usage(NULL);
-	if (logfile && check_filename(logfile, "log"))
+	if (options.logfile && check_filename(options.logfile, "log"))
 		cli_classic_abort_usage(NULL);
-	if (logfile && open_logfile(logfile))
+	if (options.logfile && open_logfile(options.logfile))
 		cli_classic_abort_usage(NULL);
 
 #if CONFIG_PRINT_WIKI == 1
-	if (list_supported_wiki) {
+	if (options.list_supported_wiki) {
 		print_supported_wiki();
 		goto out;
 	}
 #endif
 
-	if (list_supported) {
+	if (options.list_supported) {
 		if (print_supported())
 			ret = 1;
 		goto out;
@@ -927,22 +960,22 @@
 	}
 	msg_gdbg("\n");
 
-	if (layoutfile && layout_from_file(&layout, layoutfile)) {
+	if (options.layoutfile && layout_from_file(&options.layout, options.layoutfile)) {
 		ret = 1;
 		goto out;
 	}
 
-	if (!ifd && !fmap && process_include_args(layout, include_args)) {
+	if (!options.ifd && !options.fmap && process_include_args(options.layout, options.include_args)) {
 		ret = 1;
 		goto out;
 	}
 	/* Does a chip with the requested name exist in the flashchips array? */
-	if (chip_to_probe) {
+	if (options.chip_to_probe) {
 		for (chip = flashchips; chip && chip->name; chip++)
-			if (!strcmp(chip->name, chip_to_probe))
+			if (!strcmp(chip->name, options.chip_to_probe))
 				break;
 		if (!chip || !chip->name) {
-			msg_cerr("Error: Unknown chip '%s' specified.\n", chip_to_probe);
+			msg_cerr("Error: Unknown chip '%s' specified.\n", options.chip_to_probe);
 			msg_gerr("Run flashrom -L to view the hardware supported in this flashrom version.\n");
 			ret = 1;
 			goto out;
@@ -950,15 +983,15 @@
 		/* Keep chip around for later usage in case a forced read is requested. */
 	}
 
-	if (prog == NULL) {
+	if (options.prog == NULL) {
 		const struct programmer_entry *const default_programmer = CONFIG_DEFAULT_PROGRAMMER_NAME;
 
 		if (default_programmer) {
-			prog = default_programmer;
+			options.prog = default_programmer;
 			/* We need to strdup here because we free(pparam) unconditionally later. */
-			pparam = strdup(CONFIG_DEFAULT_PROGRAMMER_ARGS);
+			options.pparam = strdup(CONFIG_DEFAULT_PROGRAMMER_ARGS);
 			msg_pinfo("Using default programmer \"%s\" with arguments \"%s\".\n",
-				  default_programmer->name, pparam);
+				  default_programmer->name, options.pparam);
 		} else {
 			msg_perr("Please select a programmer with the --programmer parameter.\n"
 #if CONFIG_INTERNAL == 1
@@ -972,7 +1005,7 @@
 		}
 	}
 
-	if (programmer_init(prog, pparam)) {
+	if (programmer_init(options.prog, options.pparam)) {
 		msg_perr("Error: Programmer initialization failed.\n");
 		ret = 1;
 		goto out_shutdown;
@@ -984,7 +1017,7 @@
 	for (j = 0; j < registered_master_count; j++) {
 		startchip = 0;
 		while (chipcount < (int)ARRAY_SIZE(flashes)) {
-			startchip = probe_flash(&registered_masters[j], startchip, &flashes[chipcount], 0, chip_to_probe);
+			startchip = probe_flash(&registered_masters[j], startchip, &flashes[chipcount], 0, options.chip_to_probe);
 			if (startchip == -1)
 				break;
 			chipcount++;
@@ -1002,11 +1035,11 @@
 		goto out_shutdown;
 	} else if (!chipcount) {
 		msg_cinfo("No EEPROM/flash device found.\n");
-		if (!force || !chip_to_probe) {
+		if (!options.force || !options.chip_to_probe) {
 			msg_cinfo("Note: flashrom can never write if the flash chip isn't found "
 				  "automatically.\n");
 		}
-		if (force && read_it && chip_to_probe) {
+		if (options.force && options.read_it && options.chip_to_probe) {
 			struct registered_master *mst;
 			int compatible_masters = 0;
 			msg_cinfo("Force read (-f -r -c) requested, pretending the chip is there:\n");
@@ -1027,25 +1060,25 @@
 					  "chip, using the first one.\n");
 			for (j = 0; j < registered_master_count; j++) {
 				mst = &registered_masters[j];
-				startchip = probe_flash(mst, 0, &flashes[0], 1, chip_to_probe);
+				startchip = probe_flash(mst, 0, &flashes[0], 1, options.chip_to_probe);
 				if (startchip != -1)
 					break;
 			}
 			if (startchip == -1) {
 				// FIXME: This should never happen! Ask for a bug report?
-				msg_cinfo("Probing for flash chip '%s' failed.\n", chip_to_probe);
+				msg_cinfo("Probing for flash chip '%s' failed.\n", options.chip_to_probe);
 				ret = 1;
 				goto out_shutdown;
 			}
 			msg_cinfo("Please note that forced reads most likely contain garbage.\n");
-			flashrom_flag_set(&flashes[0], FLASHROM_FLAG_FORCE, force);
-			ret = do_read(&flashes[0], filename);
+			flashrom_flag_set(&flashes[0], FLASHROM_FLAG_FORCE, options.force);
+			ret = do_read(&flashes[0], options.filename);
 			free(flashes[0].chip);
 			goto out_shutdown;
 		}
 		ret = 1;
 		goto out_shutdown;
-	} else if (!chip_to_probe) {
+	} else if (!options.chip_to_probe) {
 		/* repeat for convenience when looking at foreign logs */
 		tempstr = flashbuses_to_text(flashes[0].chip->bustype);
 		msg_gdbg("Found %s flash chip \"%s\" (%d kB, %s).\n",
@@ -1059,13 +1092,13 @@
 	struct flashrom_progress progress_state = {
 		 .user_data = progress_user_data
 	};
-	if (show_progress)
+	if (options.show_progress)
 		flashrom_set_progress_callback(fill_flash, &flashrom_progress_cb, &progress_state);
 
 	print_chip_support_status(fill_flash->chip);
 
 	unsigned int limitexceeded = count_max_decode_exceedings(fill_flash, &max_rom_decode);
-	if (limitexceeded > 0 && !force) {
+	if (limitexceeded > 0 && !options.force) {
 		enum chipbustype commonbuses = fill_flash->mst->buses_supported & fill_flash->chip->bustype;
 
 		/* Sometimes chip and programmer have more than one bus in common,
@@ -1081,29 +1114,30 @@
 	}
 
 	const bool any_wp_op =
-		set_wp_range || set_wp_region || enable_wp ||
-		disable_wp || print_wp_status || print_wp_ranges;
+		options.set_wp_range || options.set_wp_region || options.enable_wp ||
+		options.disable_wp || options.print_wp_status || options.print_wp_ranges;
 
-	const bool any_op = read_it || write_it || verify_it || erase_it ||
-		flash_name || flash_size || extract_it || any_wp_op;
+	const bool any_op = options.read_it || options.write_it || options.verify_it ||
+		options.erase_it || options.flash_name || options.flash_size ||
+		options.extract_it || any_wp_op;
 
 	if (!any_op) {
 		msg_ginfo("No operations were specified.\n");
 		goto out_shutdown;
 	}
 
-	if (enable_wp && disable_wp) {
+	if (options.enable_wp && options.disable_wp) {
 		msg_ginfo("Error: --wp-enable and --wp-disable are mutually exclusive\n");
 		ret = 1;
 		goto out_shutdown;
 	}
-	if (set_wp_range && set_wp_region) {
+	if (options.set_wp_range && options.set_wp_region) {
 		msg_gerr("Error: Cannot use both --wp-range and --wp-region simultaneously.\n");
 		ret = 1;
 		goto out_shutdown;
 	}
 
-	if (flash_name) {
+	if (options.flash_name) {
 		if (fill_flash->chip->vendor && fill_flash->chip->name) {
 			printf("vendor=\"%s\" name=\"%s\"\n",
 				fill_flash->chip->vendor,
@@ -1114,19 +1148,19 @@
 		goto out_shutdown;
 	}
 
-	if (flash_size) {
+	if (options.flash_size) {
 		printf("%zu\n", flashrom_flash_getsize(fill_flash));
 		goto out_shutdown;
 	}
 
-	if (ifd && (flashrom_layout_read_from_ifd(&layout, fill_flash, NULL, 0) ||
-			   process_include_args(layout, include_args))) {
+	if (options.ifd && (flashrom_layout_read_from_ifd(&options.layout, fill_flash, NULL, 0) ||
+			   process_include_args(options.layout, options.include_args))) {
 		ret = 1;
 		goto out_shutdown;
-	} else if (fmap && fmapfile) {
+	} else if (options.fmap && options.fmapfile) {
 		struct stat s;
-		if (stat(fmapfile, &s) != 0) {
-			msg_gerr("Failed to stat fmapfile \"%s\"\n", fmapfile);
+		if (stat(options.fmapfile, &s) != 0) {
+			msg_gerr("Failed to stat fmapfile \"%s\"\n", options.fmapfile);
 			ret = 1;
 			goto out_shutdown;
 		}
@@ -1138,72 +1172,73 @@
 			goto out_shutdown;
 		}
 
-		if (read_buf_from_file(fmapfile_buffer, fmapfile_size, fmapfile)) {
+		if (read_buf_from_file(fmapfile_buffer, fmapfile_size, options.fmapfile)) {
 			ret = 1;
 			free(fmapfile_buffer);
 			goto out_shutdown;
 		}
 
-		if (flashrom_layout_read_fmap_from_buffer(&layout, fill_flash, fmapfile_buffer, fmapfile_size) ||
-		    process_include_args(layout, include_args)) {
+		if (flashrom_layout_read_fmap_from_buffer(&options.layout, fill_flash, fmapfile_buffer, fmapfile_size) ||
+		    process_include_args(options.layout, options.include_args)) {
 			ret = 1;
 			free(fmapfile_buffer);
 			goto out_shutdown;
 		}
 		free(fmapfile_buffer);
-	} else if (fmap && (flashrom_layout_read_fmap_from_rom(&layout, fill_flash, 0,
-				flashrom_flash_getsize(fill_flash)) || process_include_args(layout, include_args))) {
+	} else if (options.fmap && (flashrom_layout_read_fmap_from_rom(&options.layout, fill_flash, 0,
+				flashrom_flash_getsize(fill_flash)) ||
+				process_include_args(options.layout, options.include_args))) {
 		ret = 1;
 		goto out_shutdown;
 	}
-	flashrom_layout_set(fill_flash, layout);
+	flashrom_layout_set(fill_flash, options.layout);
 
 	if (any_wp_op) {
-		if (set_wp_region && wp_region) {
-			if (!layout) {
+		if (options.set_wp_region && options.wp_region) {
+			if (!options.layout) {
 				msg_gerr("Error: A flash layout must be specified to use --wp-region.\n");
 				ret = 1;
 				goto out_release;
 			}
 
-			ret = flashrom_layout_get_region_range(layout, wp_region, &wp_start, &wp_len);
+			ret = flashrom_layout_get_region_range(options.layout, options.wp_region, &options.wp_start, &options.wp_len);
 			if (ret) {
-				msg_gerr("Error: Region %s not found in flash layout.\n", wp_region);
+				msg_gerr("Error: Region %s not found in flash layout.\n", options.wp_region);
 				goto out_release;
 			}
-			set_wp_range = true;
+			options.set_wp_range = true;
 		}
 		ret = wp_cli(
 			fill_flash,
-			enable_wp,
-			disable_wp,
-			print_wp_status,
-			print_wp_ranges,
-			set_wp_range,
-			wp_start,
-			wp_len
+			options.enable_wp,
+			options.disable_wp,
+			options.print_wp_status,
+			options.print_wp_ranges,
+			options.set_wp_range,
+			options.wp_start,
+			options.wp_len
 		);
 		if (ret)
 			goto out_release;
 	}
 
-	flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, force);
+	flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, options.force);
 #if CONFIG_INTERNAL == 1
 	flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE_BOARDMISMATCH, force_boardmismatch);
 #endif
-	flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_AFTER_WRITE, !dont_verify_it);
-	flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, !dont_verify_all);
+	flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_AFTER_WRITE, !options.dont_verify_it);
+	flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, !options.dont_verify_all);
 
 	/* FIXME: We should issue an unconditional chip reset here. This can be
 	 * done once we have a .reset function in struct flashchip.
 	 * Give the chip time to settle.
 	 */
 	programmer_delay(fill_flash, 100000);
-	if (read_it)
-		ret = do_read(fill_flash, filename);
-	else if (extract_it)
+	if (options.read_it)
+		ret = do_read(fill_flash, options.filename);
+	else if (options.extract_it)
 		ret = do_extract(fill_flash);
-	else if (erase_it) {
+	else if (options.erase_it) {
 		ret = flashrom_flash_erase(fill_flash);
 		/*
 		 * FIXME: Do we really want the scary warning if erase failed?
@@ -1215,13 +1250,13 @@
 		if (ret)
 			emergency_help_message();
 	}
-	else if (write_it)
-		ret = do_write(fill_flash, filename, referencefile);
-	else if (verify_it)
-		ret = do_verify(fill_flash, filename);
+	else if (options.write_it)
+		ret = do_write(fill_flash, options.filename, options.referencefile);
+	else if (options.verify_it)
+		ret = do_verify(fill_flash, options.filename);
 
 out_release:
-	flashrom_layout_release(layout);
+	flashrom_layout_release(options.layout);
 out_shutdown:
 	flashrom_programmer_shutdown(NULL);
 out:
@@ -1230,17 +1265,7 @@
 		free(flashes[i].chip);
 	}
 
-	cleanup_include_args(&include_args);
-	free(filename);
-	free(fmapfile);
-	free(referencefile);
-	free(layoutfile);
-	free(pparam);
-	free(wp_region);
-	/* clean up global variables */
-	free((char *)chip_to_probe); /* Silence! Freeing is not modifying contents. */
-	chip_to_probe = NULL;
-	free(logfile);
+	free_options(&options);
 	ret |= close_logfile();
 	return ret;
 }

To view, visit change 66343. To unsubscribe, or for help writing mail filters, visit settings.