diff options
author | Hung-Te Lin <hungte@chromium.org> | 2019-11-22 08:29:37 +0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2019-11-22 22:09:24 +0000 |
commit | 16c91aa86c73b9013d6e1397b7d0267a74f9540a (patch) | |
tree | 2d2db2a7df5a2bef986d6ca7cfb7f720555a290b /futility | |
parent | 10c0559dd45092c26fbab9d2c9663c2d51382bae (diff) | |
download | vboot-16c91aa86c73b9013d6e1397b7d0267a74f9540a.tar.gz |
futility: updater: refactor: isolate tempfile functions from updater_config
The updater_utils.c should not deal with updater_config directly.
Currently everything relates to generating temporary files will need
updater_config due to updater_create_temp_file. By moving that out (let
every caller to pass &cfg->tempfiles) we can detach updater_utils.c from
updater_config.
BRANCH=none
BUG=chromium:1024401
TEST=make clean && make runtests
Change-Id: I44bc4df0152596a822b1e0672f41c16825472249
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1928358
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Joel Kitching <kitching@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Commit-Queue: Hung-Te Lin <hungte@chromium.org>
Auto-Submit: Hung-Te Lin <hungte@chromium.org>
Diffstat (limited to 'futility')
-rw-r--r-- | futility/updater.c | 23 | ||||
-rw-r--r-- | futility/updater.h | 3 | ||||
-rw-r--r-- | futility/updater_quirks.c | 16 | ||||
-rw-r--r-- | futility/updater_utils.c | 111 | ||||
-rw-r--r-- | futility/updater_utils.h | 76 |
5 files changed, 135 insertions, 94 deletions
diff --git a/futility/updater.c b/futility/updater.c index 14f7ccfc..21d0910b 100644 --- a/futility/updater.c +++ b/futility/updater.c @@ -376,6 +376,8 @@ static int write_firmware(struct updater_config *cfg, const struct firmware_image *image, const char *section_name) { + struct firmware_image *diff_image = NULL; + if (cfg->emulation) { INFO("(emulation) Writing %s from %s to %s (emu=%s).\n", section_name ? section_name : "whole image", @@ -383,10 +385,13 @@ static int write_firmware(struct updater_config *cfg, return emulate_write_firmware( cfg->emulation, image, section_name); - } - return write_system_firmware(cfg, image, section_name); + if (cfg->fast_update && image == &cfg->image && cfg->image_current.data) + diff_image = &cfg->image_current; + + return write_system_firmware(image, diff_image, section_name, + &cfg->tempfiles, cfg->verbosity + 1); } /* @@ -834,7 +839,7 @@ static int legacy_needs_update(struct updater_config *cfg) int has_from, has_to; const char * const tag = "cros_allow_auto_update"; const char *section = FMAP_RW_LEGACY; - const char *tmp_path = updater_create_temp_file(cfg); + const char *tmp_path = create_temp_file(&cfg->tempfiles); VB2_DEBUG("Checking %s contents...\n", FMAP_RW_LEGACY); if (!tmp_path || @@ -1170,7 +1175,8 @@ enum updater_error_codes update_firmware(struct updater_config *cfg) * RO_VPD, RW_VPD, RW_NVRAM, RW_LEGACY. */ INFO("Loading current system firmware...\n"); - if (load_system_firmware(cfg, image_from) != 0) + if (load_system_firmware(image_from, &cfg->tempfiles, + cfg->verbosity) != 0) return UPDATE_ERR_SYSTEM_IMAGE; } STATUS("Current system: %s (RO:%s, RW/A:%s, RW/B:%s).\n", @@ -1281,7 +1287,7 @@ static int updater_load_images(struct updater_config *cfg, if (!cfg->image.data && image) { if (image && strcmp(image, "-") == 0) { INFO("Reading image from stdin...\n"); - image = updater_create_temp_file(cfg); + image = create_temp_file(&cfg->tempfiles); if (image) errorcnt += !!save_file_from_stdin(image); } @@ -1336,7 +1342,7 @@ static int updater_apply_white_label(struct updater_config *cfg, assert(model->is_white_label); if (!signature_id) { if (cfg->image_current.data) { - tmp_image = updater_create_temp_file(cfg); + tmp_image = create_temp_file(&cfg->tempfiles); if (!tmp_image) return 1; if (vb2_write_file(tmp_image, cfg->image_current.data, @@ -1346,7 +1352,8 @@ static int updater_apply_white_label(struct updater_config *cfg, } } else { INFO("Loading system firmware for white label...\n"); - load_system_firmware(cfg, &cfg->image_current); + load_system_firmware(&cfg->image_current, + &cfg->tempfiles, cfg->verbosity); tmp_image = cfg->image_current.file_name; } if (!tmp_image) { @@ -1615,7 +1622,7 @@ void updater_delete_config(struct updater_config *cfg) free_firmware_image(&cfg->image_current); free_firmware_image(&cfg->ec_image); free_firmware_image(&cfg->pd_image); - updater_remove_all_temp_files(cfg); + remove_all_temp_files(&cfg->tempfiles); if (cfg->archive) archive_close(cfg->archive); free(cfg); diff --git a/futility/updater.h b/futility/updater.h index 91d3286a..c8f0f844 100644 --- a/futility/updater.h +++ b/futility/updater.h @@ -27,6 +27,7 @@ static const char * const FMAP_RO_FRID = "RO_FRID", * const FMAP_SI_DESC = "SI_DESC", * const FMAP_SI_ME = "SI_ME"; +struct updater_config; struct quirk_entry { const char *name; const char *help; @@ -59,7 +60,7 @@ struct updater_config { struct system_property system_properties[SYS_PROP_MAX]; struct quirk_entry quirks[QUIRK_MAX]; struct archive *archive; - struct tempfile *tempfiles; + struct tempfile tempfiles; int try_update; int force_update; int legacy_update; diff --git a/futility/updater_quirks.c b/futility/updater_quirks.c index aba07359..7defd577 100644 --- a/futility/updater_quirks.c +++ b/futility/updater_quirks.c @@ -107,7 +107,7 @@ static int is_ec_software_sync_enabled(struct updater_config *cfg) */ static int ec_ro_software_sync(struct updater_config *cfg) { - const char *tmp_path = updater_create_temp_file(cfg); + const char *tmp_path = create_temp_file(&cfg->tempfiles); const char *ec_ro_path; uint8_t *ec_ro_data; uint32_t ec_ro_len; @@ -124,7 +124,8 @@ static int ec_ro_software_sync(struct updater_config *cfg) ERROR("EC image has invalid section '%s'.\n", "EC_RO"); return 1; } - ec_ro_path = cbfs_extract_file(cfg, tmp_path, FMAP_RO_SECTION, "ecro"); + ec_ro_path = cbfs_extract_file(tmp_path, FMAP_RO_SECTION, "ecro", + &cfg->tempfiles); if (!ec_ro_path || !cbfs_file_exists(tmp_path, FMAP_RO_SECTION, "ecro.hash")) { INFO("No valid EC RO for software sync in AP firmware.\n"); @@ -181,7 +182,7 @@ static int quirk_enlarge_image(struct updater_config *cfg) if (image_from->size <= image_to->size) return 0; - tmp_path = updater_create_temp_file(cfg); + tmp_path = create_temp_file(&cfg->tempfiles); if (!tmp_path) return -1; @@ -348,7 +349,8 @@ static int quirk_daisy_snow_dual_model(struct updater_config *cfg) cfg->image.rw_version_a = strdup(cfg->image.rw_version_b); /* Need to use RO from current system. */ if (!cfg->image_current.data && - load_system_firmware(cfg, &cfg->image_current) != 0) { + load_system_firmware(&cfg->image_current, &cfg->tempfiles, + cfg->verbosity) != 0) { ERROR("Cannot get system RO contents\n"); return -1; } @@ -375,15 +377,15 @@ static int quirk_daisy_snow_dual_model(struct updater_config *cfg) static int quirk_eve_smm_store(struct updater_config *cfg) { const char *smm_store_name = "smm_store"; - const char *temp_image = updater_create_temp_file(cfg); + const char *temp_image = create_temp_file(&cfg->tempfiles); const char *old_store; char *command; if (write_image(temp_image, &cfg->image_current) != VB2_SUCCESS) return -1; - old_store = cbfs_extract_file(cfg, temp_image, FMAP_RW_LEGACY, - smm_store_name); + old_store = cbfs_extract_file(temp_image, FMAP_RW_LEGACY, + smm_store_name, &cfg->tempfiles); if (!old_store) { VB2_DEBUG("cbfstool failure or SMM store not available. " "Don't preserve.\n"); diff --git a/futility/updater_utils.c b/futility/updater_utils.c index 2cfb1b0c..28d41984 100644 --- a/futility/updater_utils.c +++ b/futility/updater_utils.c @@ -101,12 +101,12 @@ int cbfs_file_exists(const char *image_file, * Extracts files from a CBFS on given region (section) of image_file. * Returns the path to a temporary file on success, otherwise NULL. */ -const char *cbfs_extract_file(struct updater_config *cfg, - const char *image_file, +const char *cbfs_extract_file(const char *image_file, const char *cbfs_region, - const char *cbfs_name) + const char *cbfs_name, + struct tempfile *tempfiles) { - const char *output = updater_create_temp_file(cfg); + const char *output = create_temp_file(tempfiles); char *command, *result; if (!output) @@ -210,6 +210,25 @@ int load_firmware_image(struct firmware_image *image, const char *file_name, } /* + * Generates a temporary file for snapshot of firmware image contents. + * + * Returns a file path if success, otherwise NULL. + */ +const char *get_firmware_image_temp_file(const struct firmware_image *image, + struct tempfile *tempfiles) +{ + const char *tmp_path = create_temp_file(tempfiles); + if (!tmp_path) + return NULL; + + if (vb2_write_file(tmp_path, image->data, image->size) != VB2_SUCCESS) { + ERROR("Cannot write temporary file for output: %s\n", tmp_path); + return NULL; + } + return tmp_path; +} + +/* * Frees the allocated resource from a firmware image object. */ void free_firmware_image(struct firmware_image *image) @@ -523,19 +542,19 @@ static int host_get_wp_sw(void) * Loads the active system firmware image (usually from SPI flash chip). * Returns 0 if success, non-zero if error. */ -int load_system_firmware(struct updater_config *cfg, - struct firmware_image *image) +int load_system_firmware(struct firmware_image *image, + struct tempfile *tempfiles, int verbosity) { int r; - const char *tmp_file = updater_create_temp_file(cfg); + const char *tmp_path = create_temp_file(tempfiles); - if (!tmp_file) + if (!tmp_path) return -1; - r = host_flashrom(FLASHROM_READ, tmp_file, image->programmer, - cfg->verbosity, NULL, NULL); + r = host_flashrom(FLASHROM_READ, tmp_path, image->programmer, + verbosity, NULL, NULL); if (!r) - r = load_firmware_image(image, tmp_file, NULL); + r = load_firmware_image(image, tmp_path, NULL); return r; } @@ -544,35 +563,32 @@ int load_system_firmware(struct updater_config *cfg, * If section_name is NULL, write whole image. * Returns 0 if success, non-zero if error. */ -int write_system_firmware(struct updater_config *cfg, - const struct firmware_image *image, - const char *section_name) +int write_system_firmware(const struct firmware_image *image, + const struct firmware_image *diff_image, + const char *section_name, + struct tempfile *tempfiles, + int verbosity) { - const char *tmp_file = updater_create_temp_file(cfg); - const char *tmp_diff_file = NULL; + const char *tmp_path = get_firmware_image_temp_file(image, tempfiles); + const char *tmp_diff = NULL; + const char *programmer = image->programmer; char *extra = NULL; int r; - if (!tmp_file) + if (!tmp_path) return -1; - if (vb2_write_file(tmp_file, image->data, image->size) != VB2_SUCCESS) { - ERROR("Cannot write temporary file for output: %s\n", tmp_file); - return -1; - } - if (cfg->fast_update && image == &cfg->image && cfg->image_current.data) - { - tmp_diff_file = updater_create_temp_file(cfg); - if (vb2_write_file(tmp_diff_file, cfg->image_current.data, - cfg->image_current.size) != VB2_SUCCESS) { - ERROR("Cannot write temporary file for diff image\n"); + if (diff_image) { + tmp_diff = get_firmware_image_temp_file( + diff_image, tempfiles); + if (!tmp_diff) return -1; - } - ASPRINTF(&extra, "--noverify --diff=%s", tmp_diff_file); + ASPRINTF(&extra, "--noverify --diff=%s", tmp_diff); } - r = host_flashrom(FLASHROM_WRITE, tmp_file, programmer, - cfg->verbosity + 1, section_name, extra); + + r = host_flashrom(FLASHROM_WRITE, tmp_path, programmer, verbosity, + section_name, extra); free(extra); return r; } @@ -592,10 +608,10 @@ void init_system_properties(struct system_property *props, int num) /* * Helper function to create a new temporary file. - * All files created will be removed by updater_remove_all_temp_files(). + * All files created will be removed remove_all_temp_files(). * Returns the path of new file, or NULL on failure. */ -const char *updater_create_temp_file(struct updater_config *cfg) +const char *create_temp_file(struct tempfile *head) { struct tempfile *new_temp; char new_path[] = P_tmpdir "/fwupdater.XXXXXX"; @@ -617,8 +633,10 @@ const char *updater_create_temp_file(struct updater_config *cfg) return NULL; } VB2_DEBUG("Created new temporary file: %s.\n", new_path); - new_temp->next = cfg->tempfiles; - cfg->tempfiles = new_temp; + new_temp->next = NULL; + while (head->next) + head = head->next; + head->next = new_temp; return new_temp->filepath; } @@ -626,16 +644,19 @@ const char *updater_create_temp_file(struct updater_config *cfg) * Helper function to remove all files created by create_temp_file(). * This is intended to be called only once at end of program execution. */ -void updater_remove_all_temp_files(struct updater_config *cfg) +void remove_all_temp_files(struct tempfile *head) { - struct tempfile *tempfiles = cfg->tempfiles; - while (tempfiles != NULL) { - struct tempfile *target = tempfiles; - VB2_DEBUG("Remove temporary file: %s.\n", target->filepath); - remove(target->filepath); - free(target->filepath); - tempfiles = target->next; - free(target); + /* head itself is dummy and should not be removed. */ + assert(!head->filepath); + struct tempfile *next = head->next; + while (next) { + head->next = NULL; + head = next; + next = head->next; + assert(head->filepath); + VB2_DEBUG("Remove temporary file: %s.\n", head->filepath); + remove(head->filepath); + free(head->filepath); + free(head); } - cfg->tempfiles = NULL; } diff --git a/futility/updater_utils.h b/futility/updater_utils.h index 686b0888..eb455d13 100644 --- a/futility/updater_utils.h +++ b/futility/updater_utils.h @@ -14,9 +14,6 @@ #define ASPRINTF(strp, ...) do { if (asprintf(strp, __VA_ARGS__) >= 0) break; \ ERROR("Failed to allocate memory, abort.\n"); exit(1); } while (0) -/* Structure(s) declared in updater.h */ -struct updater_config; - /* Structure(s) declared in updater_archive */ struct archive; @@ -35,6 +32,28 @@ enum active_slot { SLOT_B, }; +/* Utilities for managing temporary files. */ +struct tempfile { + char *filepath; + struct tempfile *next; +}; + +/* + * Create a new temporary file. + * + * The parameter head refers to a linked list dummy head. + * Returns the path of new file, or NULL on failure. + */ +const char *create_temp_file(struct tempfile *head); + +/* + * Remove all files created by create_temp_file(). + * + * The parameter head refers to the dummy head of linked list. + * This is intended to be called only once at end of program execution. + */ +void remove_all_temp_files(struct tempfile *head); + /* Utilities for firmware images and (FMAP) sections */ struct firmware_image { const char *programmer; @@ -58,20 +77,30 @@ int load_firmware_image(struct firmware_image *image, const char *file_name, * Loads the active system firmware image (usually from SPI flash chip). * Returns 0 if success, non-zero if error. */ -int load_system_firmware(struct updater_config *cfg, - struct firmware_image *image); +int load_system_firmware(struct firmware_image *image, + struct tempfile *tempfiles, int verbosity); + +/* Frees the allocated resource from a firmware image object. */ +void free_firmware_image(struct firmware_image *image); + +/* + * Generates a temporary file for snapshot of firmware image contents. + * + * Returns a file path if success, otherwise NULL. + */ +const char *get_firmware_image_temp_file(const struct firmware_image *image, + struct tempfile *tempfiles); /* * Writes a section from given firmware image to system firmware. * If section_name is NULL, write whole image. * Returns 0 if success, non-zero if error. */ -int write_system_firmware(struct updater_config *cfg, - const struct firmware_image *image, - const char *section_name); - -/* Frees the allocated resource from a firmware image object. */ -void free_firmware_image(struct firmware_image *image); +int write_system_firmware(const struct firmware_image *image, + const struct firmware_image *diff_image, + const char *section_name, + struct tempfile *tempfiles, + int verbosity); struct firmware_section { uint8_t *data; @@ -153,10 +182,10 @@ int cbfs_file_exists(const char *image_file, * Extracts files from a CBFS on given region (section) of image_file. * Returns the path to a temporary file on success, otherwise NULL. */ -const char *cbfs_extract_file(struct updater_config *cfg, - const char *image_file, +const char *cbfs_extract_file(const char *image_file, const char *cbfs_region, - const char *cbfs_name); + const char *cbfs_name, + struct tempfile *tempfiles); /* Utilities for accessing system properties */ struct system_property { @@ -178,23 +207,4 @@ enum system_property_type { /* Helper function to initialize system properties. */ void init_system_properties(struct system_property *props, int num); -/* Utilities for managing temporary files. */ -/* TODO(hungte) Change the functions below to take only tempfile as param. */ -struct tempfile { - char *filepath; - struct tempfile *next; -}; - -/* - * Helper function to create a new temporary file within updater's life cycle. - * Returns the path of new file, or NULL on failure. - */ -const char *updater_create_temp_file(struct updater_config *cfg); - -/* - * Helper function to remove all files created by create_temp_file(). - * This is intended to be called only once at end of program execution. - */ -void updater_remove_all_temp_files(struct updater_config *cfg); - #endif /* VBOOT_REFERENCE_FUTILITY_UPDATER_UTILS_H_ */ |