From 7c94d5be063a5950b8af558739b7c351eddd3ced Mon Sep 17 00:00:00 2001 From: Hung-Te Lin Date: Sat, 6 Oct 2018 01:14:19 +0800 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/1265575 Reviewed-by: Randall Spangler --- futility/cmd_update.c | 54 ++++++----------- futility/updater.c | 165 +++++++++++++++++++++++++++----------------------- futility/updater.h | 24 +++----- 3 files changed, 118 insertions(+), 125 deletions(-) (limited to 'futility') 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 @@ -1630,111 +1630,124 @@ static int save_from_stdin(const char *output) return 0; } +/* + * 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); -- cgit v1.2.1