futility: updater: Refactor how arguments were configured

We are going to have more command line arguments that must be passed to
updater_setup_config, and it is better to manage so many variables in a
struct.

Also, revised the order or argument processing so that simple settings
are now processed first, then complicated ones or those with dependency.

BUG=chromium:875551
TEST=TEST=make futil; tests/futility/run_test_scripts.sh $(pwd)/build/futility
BRANCH=None

Change-Id: I03ac036d26e49cdf924c03d6e86a272ce89fc2aa
Signed-off-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1265575
Reviewed-by: Randall Spangler <rspangler@chromium.org>

3 files changed, 118 insertions, 125 deletions

diff --git a/futility/cmd_update.c b/futility/cmd_update.c
index 830f9ace..df09dc8c 100644
--- a/futility/cmd_update.c
+++ b/futility/cmd_update.c
@@ -71,20 +71,9 @@ static void print_help(int argc, char *argv[])
 
 static int do_update(int argc, char *argv[])
 {
-	const char *opt_image = NULL,
-	           *opt_ec_image = NULL,
-		   *opt_pd_image = NULL,
-		   *opt_archive = NULL,
-		   *opt_quirks = NULL,
-		   *opt_mode = NULL,
-		   *opt_programmer = NULL,
-		   *opt_emulation = NULL,
-		   *opt_sys_props = NULL,
-		   *opt_write_protection = NULL;
-	int opt_is_factory = 0, opt_try_update = 0, opt_force_update = 0,
-	    opt_verbosity = 0;
-	int i, errorcnt = 0;
 	struct updater_config *cfg;
+	struct updater_config_arguments args = {0};
+	int i, errorcnt = 0;
 
 	printf(">> Firmware updater started.\n");
 	cfg = updater_new_config();
@@ -94,53 +83,54 @@ static int do_update(int argc, char *argv[])
 	while ((i = getopt_long(argc, argv, short_opts, long_opts, 0)) != -1) {
 		switch (i) {
 		case 'i':
-			opt_image = optarg;
+			args.image = optarg;
 			break;
 		case 'e':
-			opt_ec_image = optarg;
+			args.ec_image = optarg;
 			break;
 		case 'P':
-			opt_pd_image = optarg;
+			args.pd_image = optarg;
 			break;
 		case 't':
-			opt_try_update = 1;
+			args.try_update = 1;
 			break;
 		case 'a':
-			opt_archive = optarg;
+			args.archive = optarg;
 			break;
 		case 'f':
-			opt_quirks = optarg;
+			args.quirks = optarg;
 			break;
 		case 'L':
 			updater_list_config_quirks(cfg);
 			return 0;
 		case 'm':
-			opt_mode = optarg;
+			args.mode = optarg;
+			break;
 			break;
 		case 'Y':
-			opt_is_factory = 1;
+			args.is_factory = 1;
 			break;
 		case 'W':
-			opt_write_protection = optarg;
+			args.write_protection = optarg;
 			break;
 		case 'E':
-			opt_emulation = optarg;
+			args.emulation = optarg;
 			break;
 		case 'p':
-			opt_programmer = optarg;
+			args.programmer = optarg;
 			break;
 		case 'F':
-			opt_force_update = 1;
+			args.force_update = 1;
 			break;
 		case 'S':
-			opt_sys_props = optarg;
+			args.sys_props = optarg;
 			break;
 		case 'v':
-			opt_verbosity++;
+			args.verbosity++;
 			break;
 		case 'd':
 			debugging_enabled = 1;
-			opt_verbosity++;
+			args.verbosity++;
 			break;
 
 		case 'h':
@@ -166,13 +156,7 @@ static int do_update(int argc, char *argv[])
 		Error("Unexpected arguments.\n");
 	}
 	if (!errorcnt)
-		errorcnt += updater_setup_config(
-				cfg, opt_image, opt_ec_image, opt_pd_image,
-				opt_archive, opt_quirks, opt_mode,
-				opt_programmer, opt_emulation, opt_sys_props,
-				opt_write_protection, opt_is_factory,
-				opt_try_update, opt_force_update,
-				opt_verbosity);
+		errorcnt += updater_setup_config(cfg, &args);
 	if (!errorcnt) {
 		int r = update_firmware(cfg);
 		if (r != UPDATE_ERR_DONE) {
diff --git a/futility/updater.c b/futility/updater.c
index b9e939b1..95ec6612 100644
--- a/futility/updater.c
+++ b/futility/updater.c
@@ -1631,110 +1631,123 @@ static int save_from_stdin(const char *output)
 }
 
 /*
+ * Loads images into updater configuration.
+ * Returns 0 on success, otherwise number of failures.
+ */
+static int updater_load_images(struct updater_config *cfg,
+			       const char *image,
+			       const char *ec_image,
+			       const char *pd_image)
+{
+	int errorcnt = 0;
+	struct archive *ar = cfg->archive;
+
+	if (!cfg->image.data && image) {
+		if (image && strcmp(image, "-") == 0) {
+			fprintf(stderr, "Reading image from stdin...\n");
+			image = updater_create_temp_file(cfg);
+			if (image)
+				errorcnt += !!save_from_stdin(image);
+		}
+		errorcnt += !!load_firmware_image(&cfg->image, image, ar);
+	}
+	if (cfg->emulation)
+		return errorcnt;
+
+	if (!cfg->ec_image.data && ec_image)
+		errorcnt += !!load_firmware_image(&cfg->ec_image, ec_image, ar);
+	if (!cfg->pd_image.data && pd_image)
+		errorcnt += !!load_firmware_image(&cfg->pd_image, pd_image, ar);
+	return errorcnt;
+}
+
+/*
  * Helper function to setup an allocated updater_config object.
  * Returns number of failures, or 0 on success.
  */
 int updater_setup_config(struct updater_config *cfg,
-			 const char *image,
-			 const char *ec_image,
-			 const char *pd_image,
-			 const char *archive,
-			 const char *quirks,
-			 const char *mode,
-			 const char *programmer,
-			 const char *emulation,
-			 const char *sys_props,
-			 const char *write_protection,
-			 int is_factory,
-			 int try_update,
-			 int force_update,
-			 int verbosity)
+			 const struct updater_config_arguments *arg)
 {
 	int errorcnt = 0;
 	int check_single_image = 0, check_wp_disabled = 0;
 	const char *default_quirks = NULL;
-	struct archive *ar;
+	int is_factory = arg->is_factory;
+	const char *archive_path = arg->archive;
 
-	cfg->verbosity = verbosity;
-
-	cfg->archive = archive_open(archive ? archive : ".");
-	if (!cfg->archive) {
-		ERROR("Failed to open archive: %s", archive);
-		return ++errorcnt;
-	}
-	ar = cfg->archive;
-
-	if (try_update)
-		cfg->try_update = 1;
-	if (force_update)
+	/* Setup values that may change output or decision of other argument. */
+	cfg->verbosity = arg->verbosity;
+	if (arg->force_update)
 		cfg->force_update = 1;
 
-	if (sys_props)
-		override_properties_from_list(sys_props, cfg);
-
-	if (image && strcmp(image, "-") == 0) {
-		fprintf(stderr, "Reading image from stdin...\n");
-		image = updater_create_temp_file(cfg);
-		if (image)
-			errorcnt += !!save_from_stdin(image);
-	}
-	if (image) {
-		errorcnt += !!load_firmware_image(&cfg->image, image, ar);
-	}
-	if (ec_image)
-		errorcnt += !!load_firmware_image(&cfg->ec_image, ec_image, ar);
-	if (pd_image)
-		errorcnt += !!load_firmware_image(&cfg->pd_image, pd_image, ar);
-
-	/*
-	 * Quirks must be loaded after images are loaded because we use image
-	 * contents to decide default quirks to load. Also, we have to load
-	 * default quirks first so user can override them using command line.
-	 */
-	default_quirks = updater_get_default_quirks(cfg);
-	if (default_quirks)
-		errorcnt += !!setup_config_quirks(default_quirks, cfg);
-	if (quirks)
-		errorcnt += !!setup_config_quirks(quirks, cfg);
-
-	if (mode) {
-		if (strcmp(mode, "autoupdate") == 0) {
+	/* Setup update mode. */
+	if (arg->try_update)
+		cfg->try_update = 1;
+	if (arg->mode) {
+		if (strcmp(arg->mode, "autoupdate") == 0) {
 			cfg->try_update = 1;
-		} else if (strcmp(mode, "recovery") == 0) {
+		} else if (strcmp(arg->mode, "recovery") == 0) {
 			cfg->try_update = 0;
-		} else if (strcmp(mode, "legacy") == 0) {
+		} else if (strcmp(arg->mode, "legacy") == 0) {
 			cfg->legacy_update = 1;
-		} else if (strcmp(mode, "factory") == 0 ||
-			   strcmp(mode, "factory_install") == 0) {
+		} else if (strcmp(arg->mode, "factory") == 0 ||
+			   strcmp(arg->mode, "factory_install") == 0) {
 			is_factory = 1;
 		} else {
 			errorcnt++;
-			ERROR("Invalid mode: %s", mode);
+			ERROR("Invalid mode: %s", arg->mode);
 		}
 	}
-	/* Must be checked after mode selection. */
 	if (is_factory) {
+		/* is_factory must be processed after arg->mode. */
 		check_wp_disabled = 1;
 		cfg->try_update = 0;
 	}
-	if (write_protection) {
-		int r = strtol(write_protection, NULL, 0);
+
+	/* Setup properties and fields that do not have external dependency. */
+	if (arg->programmer) {
+		check_single_image = 1;
+		cfg->image.programmer = arg->programmer;
+		cfg->image_current.programmer = arg->programmer;
+		DEBUG("AP (host) programmer changed to %s.", arg->programmer);
+	}
+	if (arg->sys_props)
+		override_properties_from_list(arg->sys_props, cfg);
+	if (arg->write_protection) {
+		/* arg->write_protection must be done after arg->sys_props. */
+		int r = strtol(arg->write_protection, NULL, 0);
 		override_system_property(SYS_PROP_WP_HW, cfg, r);
 		override_system_property(SYS_PROP_WP_SW, cfg, r);
 	}
-	if (programmer) {
-		check_single_image = 1;
-		cfg->image.programmer = programmer;
-		cfg->image_current.programmer = programmer;
-		DEBUG("AP (host) programmer changed to %s.", programmer);
-	}
-	if (emulation) {
+
+	/* Set up archive and load images. */
+	if (arg->emulation) {
+		/* Process emulation file first. */
 		check_single_image = 1;
-		cfg->emulation = emulation;
-		DEBUG("Using file %s for emulation.", emulation);
-		errorcnt += load_firmware_image(
-				&cfg->image_current, emulation, NULL);
+		cfg->emulation = arg->emulation;
+		DEBUG("Using file %s for emulation.", arg->emulation);
+		errorcnt += !!load_firmware_image(
+				&cfg->image_current, arg->emulation, NULL);
+	}
+	if (!archive_path)
+		archive_path = ".";
+	cfg->archive = archive_open(archive_path);
+	if (!cfg->archive) {
+		ERROR("Failed to open archive: %s", archive_path);
+		return ++errorcnt;
 	}
+	errorcnt += updater_load_images(
+			cfg, arg->image, arg->ec_image, arg->pd_image);
+
+	/*
+	 * Quirks must be loaded after images are loaded because we use image
+	 * contents to decide default quirks to load. Also, we have to load
+	 * default quirks first so user can override them using command line.
+	 */
+	default_quirks = updater_get_default_quirks(cfg);
+	if (default_quirks)
+		errorcnt += !!setup_config_quirks(default_quirks, cfg);
+	if (arg->quirks)
+		errorcnt += !!setup_config_quirks(arg->quirks, cfg);
 
 	/* Additional checks. */
 	if (check_single_image && (cfg->ec_image.data || cfg->pd_image.data)) {
diff --git a/futility/updater.h b/futility/updater.h
index c04e9c3c..a6cb3a72 100644
--- a/futility/updater.h
+++ b/futility/updater.h
@@ -109,6 +109,15 @@ struct updater_config {
 	const char *emulation;
 };
 
+struct updater_config_arguments {
+	char *image, *ec_image, *pd_image;
+	char *archive, *quirks, *mode;
+	char *programmer, *model;
+	char *emulation, *sys_props, *write_protection;
+	int is_factory, try_update, force_update, do_manifest;
+	int verbosity;
+};
+
 enum updater_error_codes {
 	UPDATE_ERR_DONE,
 	UPDATE_ERR_NEED_RO_UPDATE,
@@ -151,20 +160,7 @@ void updater_delete_config(struct updater_config *cfg);
  * Returns number of failures, or 0 on success.
  */
 int updater_setup_config(struct updater_config *cfg,
-			  const char *image,
-			  const char *ec_image,
-			  const char *pd_image,
-			  const char *archive,
-			  const char *quirks,
-			  const char *mode,
-			  const char *programmer,
-			  const char *emulation,
-			  const char *sys_props,
-			  const char *write_protection,
-			  int is_factory,
-			  int try_update,
-			  int force_update,
-			  int verbosity);
+			 const struct updater_config_arguments *arg);
 
 /* Prints the name and description from all supported quirks. */
 void updater_list_config_quirks(const struct updater_config *cfg);