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>

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_ */