vboot_reference: lib/flashrom convert args into struct

This attempts to converge the signatures of host/lib/flashrom
with that of 'futility/updater_utils.h:struct firmware_image'.
with the eventual goal of converging the multiple flashrom
wrapper implementations.

BUG=b:207808292
BRANCH=none
TEST=`$ cros_run_unit_tests --board nocturne --packages vboot_reference`

Cq-Depend: chromium:3399963
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Change-Id: I016dacbdca6f1108def0dbc608d83e0066a30023
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3301571
Reviewed-by: Sam McNally <sammc@chromium.org>
Tested-by: Edward O'Callaghan <quasisec@chromium.org>
Auto-Submit: Edward O'Callaghan <quasisec@chromium.org>
Commit-Queue: Edward O'Callaghan <quasisec@chromium.org>

5 files changed, 92 insertions, 79 deletions

diff --git a/host/lib/crossystem.c b/host/lib/crossystem.c
index 7c3ee879..a5427c2d 100644
--- a/host/lib/crossystem.c
+++ b/host/lib/crossystem.c
@@ -821,21 +821,21 @@ int vb2_read_nv_storage_flashrom(struct vb2_context *ctx)
 {
 	int index;
 	int vbnv_size = vb2_nv_get_size(ctx);
-	uint8_t *flash_buf;
-	uint32_t flash_size;
 
-	if (flashrom_read(FLASHROM_PROGRAMMER_INTERNAL_AP, VBNV_FMAP_REGION,
-			  &flash_buf, &flash_size))
+	struct firmware_image image = {
+		.programmer = FLASHROM_PROGRAMMER_INTERNAL_AP,
+	};
+	if (flashrom_read(&image, VBNV_FMAP_REGION))
 		return -1;
 
-	index = vb2_nv_index(flash_buf, flash_size, vbnv_size);
+	index = vb2_nv_index(image.data, image.size, vbnv_size);
 	if (index < 0) {
-		free(flash_buf);
+		free(image.data);
 		return -1;
 	}
 
-	memcpy(ctx->nvdata, &flash_buf[index * vbnv_size], vbnv_size);
-	free(flash_buf);
+	memcpy(ctx->nvdata, &image.data[index * vbnv_size], vbnv_size);
+	free(image.data);
 	return 0;
 }
 
@@ -845,34 +845,33 @@ int vb2_write_nv_storage_flashrom(struct vb2_context *ctx)
 	int current_index;
 	int next_index;
 	int vbnv_size = vb2_nv_get_size(ctx);
-	uint8_t *flash_buf;
-	uint32_t flash_size;
 
-	if (flashrom_read(FLASHROM_PROGRAMMER_INTERNAL_AP, VBNV_FMAP_REGION,
-			  &flash_buf, &flash_size))
+	struct firmware_image image = {
+		.programmer = FLASHROM_PROGRAMMER_INTERNAL_AP,
+	};
+	if (flashrom_read(&image, VBNV_FMAP_REGION))
 		return -1;
 
-	current_index = vb2_nv_index(flash_buf, flash_size, vbnv_size);
+	current_index = vb2_nv_index(image.data, image.size, vbnv_size);
 	if (current_index < 0) {
 		rv = -1;
 		goto exit;
 	}
 
 	next_index = current_index + 1;
-	if (next_index * vbnv_size == flash_size) {
+	if (next_index * vbnv_size == image.size) {
 		/* VBNV is full.  Erase and write at beginning. */
-		memset(flash_buf, 0xff, flash_size);
+		memset(image.data, 0xff, image.size);
 		next_index = 0;
 	}
 
-	memcpy(&flash_buf[next_index * vbnv_size], ctx->nvdata, vbnv_size);
-	if (flashrom_write(FLASHROM_PROGRAMMER_INTERNAL_AP, VBNV_FMAP_REGION,
-			   flash_buf, flash_size)) {
+	memcpy(&image.data[next_index * vbnv_size], ctx->nvdata, vbnv_size);
+	if (flashrom_write(&image, VBNV_FMAP_REGION)) {
 		rv = -1;
 		goto exit;
 	}
 
  exit:
-	free(flash_buf);
+	free(image.data);
 	return rv;
 }
diff --git a/host/lib/flashrom.c b/host/lib/flashrom.c
index e83dfb20..6ee5971b 100644
--- a/host/lib/flashrom.c
+++ b/host/lib/flashrom.c
@@ -103,15 +103,14 @@ static vb2_error_t run_flashrom(const char *const argv[])
 	return VB2_SUCCESS;
 }
 
-vb2_error_t flashrom_read(const char *programmer, const char *region,
-			  uint8_t **data_out, uint32_t *size_out)
+vb2_error_t flashrom_read(struct firmware_image *image, const char *region)
 {
 	char *tmpfile;
 	char region_param[PATH_MAX];
 	vb2_error_t rv;
 
-	*data_out = NULL;
-	*size_out = 0;
+	image->data = NULL;
+	image->size = 0;
 
 	VB2_TRY(write_temp_file(NULL, 0, &tmpfile));
 
@@ -122,7 +121,7 @@ vb2_error_t flashrom_read(const char *programmer, const char *region,
 	const char *const argv[] = {
 		FLASHROM_EXEC_NAME,
 		"-p",
-		programmer,
+		image->programmer,
 		"-r",
 		region ? "-i" : tmpfile,
 		region ? region_param : NULL,
@@ -131,21 +130,20 @@ vb2_error_t flashrom_read(const char *programmer, const char *region,
 
 	rv = run_flashrom(argv);
 	if (rv == VB2_SUCCESS)
-		rv = vb2_read_file(tmpfile, data_out, size_out);
+		rv = vb2_read_file(tmpfile, &image->data, &image->size);
 
 	unlink(tmpfile);
 	free(tmpfile);
 	return rv;
 }
 
-vb2_error_t flashrom_write(const char *programmer, const char *region,
-			   uint8_t *data, uint32_t size)
+vb2_error_t flashrom_write(struct firmware_image *image, const char *region)
 {
 	char *tmpfile;
 	char region_param[PATH_MAX];
 	vb2_error_t rv;
 
-	VB2_TRY(write_temp_file(data, size, &tmpfile));
+	VB2_TRY(write_temp_file(image->data, image->size, &tmpfile));
 
 	if (region)
 		snprintf(region_param, sizeof(region_param), "%s:%s", region,
@@ -154,7 +152,7 @@ vb2_error_t flashrom_write(const char *programmer, const char *region,
 	const char *const argv[] = {
 		FLASHROM_EXEC_NAME,
 		"-p",
-		programmer,
+		image->programmer,
 		"--noverify-all",
 		"-w",
 		region ? "-i" : tmpfile,
diff --git a/host/lib/include/flashrom.h b/host/lib/include/flashrom.h
index 560fbb0e..0d934424 100644
--- a/host/lib/include/flashrom.h
+++ b/host/lib/include/flashrom.h
@@ -12,39 +12,39 @@
 #define FLASHROM_PROGRAMMER_INTERNAL_AP "host"
 #define FLASHROM_PROGRAMMER_INTERNAL_EC "ec"
 
+/* Utilities for firmware images and (FMAP) sections */
+struct firmware_image {
+	/**
+	 * programmer	The name of the programmer to use. Use either
+	 *		FLASHROM_PROGRAMMER_INTERNAL_AP or,
+	 *		FLASHROM_PROGRAMMER_INTERNAL_EC
+	 *		for the AP and EC respectively.
+	 */
+	const char *programmer;
+	uint32_t size; /* buffer size. */
+	uint8_t *data; /* data allocated buffer to read/write with. */
+};
+
 /**
  * Read using flashrom into an allocated buffer.
  *
- * @param programmer	The name of the programmer to use.  There are
- *			named constants FLASHROM_PROGRAMMER_INTERNAL_AP
- *			and FLASHROM_PROGRAMMER_INTERNAL_EC available
- *			for the AP and EC respectively, or a custom
- *			programmer string can be provided.
+ * @param image		The parameter that contains the programmer, buffer and
+ *			size to use in the read operation.
  * @param region	The name of the fmap region to read, or NULL to
  *			read the entire flash chip.
- * @param data_out	Output parameter of allocated buffer to read into.
- *			The caller should free the buffer.
- * @param size_out	Output parameter of buffer size.
  *
  * @return VB2_SUCCESS on success, or a relevant error.
  */
-vb2_error_t flashrom_read(const char *programmer, const char *region,
-			  uint8_t **data_out, uint32_t *size_out);
+vb2_error_t flashrom_read(struct firmware_image *image, const char *region);
 
 /**
  * Write using flashrom from a buffer.
  *
- * @param programmer	The name of the programmer to use.  There are
- *			named constants FLASHROM_PROGRAMMER_INTERNAL_AP
- *			and FLASHROM_PROGRAMMER_INTERNAL_EC available
- *			for the AP and EC respectively, or a custom
- *			programmer string can be provided.
+ * @param image		The parameter that contains the programmer, buffer and
+ *			size to use in the write operation.
  * @param region	The name of the fmap region to write, or NULL to
  *			write the entire flash chip.
- * @param data		The buffer to write.
- * @param size		The size of the buffer to write.
  *
  * @return VB2_SUCCESS on success, or a relevant error.
  */
-vb2_error_t flashrom_write(const char *programmer, const char *region,
-			   uint8_t *data, uint32_t size);
+vb2_error_t flashrom_write(struct firmware_image *image, const char *region);
diff --git a/tests/vb2_host_flashrom_tests.c b/tests/vb2_host_flashrom_tests.c
index 8552d0ad..815657d4 100644
--- a/tests/vb2_host_flashrom_tests.c
+++ b/tests/vb2_host_flashrom_tests.c
@@ -145,10 +145,11 @@ int subprocess_run(const char *const argv[],
 
 static void test_read_whole_chip(void)
 {
-	uint8_t *buf;
-	uint32_t buf_sz;
+	struct firmware_image image = {
+		.programmer = "someprog",
+	};
 
-	TEST_SUCC(flashrom_read("someprog", NULL, &buf, &buf_sz),
+	TEST_SUCC(flashrom_read(&image, NULL),
 		  "Flashrom read succeeds");
 	TEST_STR_EQ(captured_programmer, "someprog",
 		    "Using specified programmer");
@@ -158,19 +159,20 @@ static void test_read_whole_chip(void)
 	TEST_STR_EQ(captured_op_filename, MOCK_TMPFILE_NAME,
 		    "Reading to correct file");
 	TEST_PTR_EQ(captured_region_param, NULL, "Not operating on a region");
-	TEST_EQ(buf_sz, strlen(MOCK_ROM_CONTENTS), "Contents correct size");
-	TEST_SUCC(memcmp(buf, MOCK_ROM_CONTENTS, buf_sz),
+	TEST_EQ(image.size, strlen(MOCK_ROM_CONTENTS), "Contents correct size");
+	TEST_SUCC(memcmp(image.data, MOCK_ROM_CONTENTS, image.size),
 		  "Buffer has correct contents");
 
-	free(buf);
+	free(image.data);
 }
 
 static void test_read_region(void)
 {
-	uint8_t *buf;
-	uint32_t buf_sz;
+	struct firmware_image image = {
+		.programmer = "someprog",
+	};
 
-	TEST_SUCC(flashrom_read("someprog", "SOME_REGION", &buf, &buf_sz),
+	TEST_SUCC(flashrom_read(&image, "SOME_REGION"),
 		  "Flashrom read succeeds");
 	TEST_STR_EQ(captured_programmer, "someprog",
 		    "Using specified programmer");
@@ -181,20 +183,21 @@ static void test_read_region(void)
 		    "Not doing a read of the whole ROM");
 	TEST_STR_EQ(captured_region_param, "SOME_REGION:" MOCK_TMPFILE_NAME,
 		    "Reading to correct file and from correct region");
-	TEST_EQ(buf_sz, strlen(MOCK_ROM_CONTENTS), "Contents correct size");
-	TEST_SUCC(memcmp(buf, MOCK_ROM_CONTENTS, buf_sz),
+	TEST_EQ(image.size, strlen(MOCK_ROM_CONTENTS), "Contents correct size");
+	TEST_SUCC(memcmp(image.data, MOCK_ROM_CONTENTS, image.size),
 		  "Buffer has correct contents");
 
-	free(buf);
+	free(image.data);
 }
 
 static void test_read_failure(void)
 {
-	uint8_t *buf;
-	uint32_t buf_sz;
+	struct firmware_image image = {
+		.programmer = "someprog",
+	};
 
 	flashrom_mock_success = false;
-	TEST_NEQ(flashrom_read("someprog", "SOME_REGION", &buf, &buf_sz),
+	TEST_NEQ(flashrom_read(&image, "SOME_REGION"),
 		 VB2_SUCCESS, "Flashrom read fails");
 	flashrom_mock_success = true;
 }
@@ -202,10 +205,15 @@ static void test_read_failure(void)
 static void test_write_whole_chip(void)
 {
 	uint8_t buf[sizeof(MOCK_ROM_CONTENTS) - 1];
+	struct firmware_image image = {
+		.programmer = "someprog",
+		.data = buf,
+		.size = sizeof(buf),
+	};
 
 	memcpy(buf, MOCK_ROM_CONTENTS, sizeof(buf));
 
-	TEST_SUCC(flashrom_write("someprog", NULL, buf, sizeof(buf)),
+	TEST_SUCC(flashrom_write(&image, NULL),
 		  "Flashrom write succeeds");
 	TEST_STR_EQ(captured_programmer, "someprog",
 		    "Using specified programmer");
@@ -224,10 +232,15 @@ static void test_write_whole_chip(void)
 static void test_write_region(void)
 {
 	uint8_t buf[sizeof(MOCK_ROM_CONTENTS) - 1];
+	struct firmware_image image = {
+		.programmer = "someprog",
+		.data = buf,
+		.size = sizeof(buf),
+	};
 
 	memcpy(buf, MOCK_ROM_CONTENTS, sizeof(buf));
 
-	TEST_SUCC(flashrom_write("someprog", "SOME_REGION", buf, sizeof(buf)),
+	TEST_SUCC(flashrom_write(&image, "SOME_REGION"),
 		  "Flashrom write succeeds");
 	TEST_STR_EQ(captured_programmer, "someprog",
 		    "Using specified programmer");
@@ -247,9 +260,14 @@ static void test_write_region(void)
 static void test_write_failure(void)
 {
 	uint8_t buf[20] = { 0 };
+	struct firmware_image image = {
+		.programmer = "someprog",
+		.data = buf,
+		.size = sizeof(buf),
+	};
 
 	flashrom_mock_success = false;
-	TEST_NEQ(flashrom_write("someprog", "SOME_REGION", buf, sizeof(buf)),
+	TEST_NEQ(flashrom_write(&image, "SOME_REGION"),
 		 VB2_SUCCESS, "Flashrom write fails");
 	flashrom_mock_success = true;
 }
diff --git a/tests/vb2_host_nvdata_flashrom_tests.c b/tests/vb2_host_nvdata_flashrom_tests.c
index 33b435d9..068b23a4 100644
--- a/tests/vb2_host_nvdata_flashrom_tests.c
+++ b/tests/vb2_host_nvdata_flashrom_tests.c
@@ -81,35 +81,33 @@ static void reset_test_data(struct vb2_context *ctx, int nvdata_size)
 }
 
 /* Mocked flashrom_read for tests. */
-vb2_error_t flashrom_read(const char *programmer, const char *region,
-			  uint8_t **data_out, uint32_t *size_out)
+vb2_error_t flashrom_read(struct firmware_image *image, const char *region)
 {
 	if (mock_flashrom_fail) {
-		*data_out = NULL;
-		*size_out = 0;
+		image->data = NULL;
+		image->size = 0;
 		return VB2_ERROR_FLASHROM;
 	}
 
-	assert_mock_params(programmer, region);
+	assert_mock_params(image->programmer, region);
 
-	*data_out = malloc(sizeof(fake_flash_region));
-	*size_out = sizeof(fake_flash_region);
-	memcpy(*data_out, fake_flash_region, sizeof(fake_flash_region));
+	image->data = malloc(sizeof(fake_flash_region));
+	image->size = sizeof(fake_flash_region);
+	memcpy(image->data, fake_flash_region, sizeof(fake_flash_region));
 	return VB2_SUCCESS;
 }
 
 /* Mocked flashrom_write for tests. */
-vb2_error_t flashrom_write(const char *programmer, const char *region,
-			   uint8_t *data, uint32_t data_size)
+vb2_error_t flashrom_write(struct firmware_image *image, const char *region)
 {
 	if (mock_flashrom_fail)
 		return VB2_ERROR_FLASHROM;
 
-	assert_mock_params(programmer, region);
+	assert_mock_params(image->programmer, region);
 
-	TEST_EQ(data_size, sizeof(fake_flash_region),
+	TEST_EQ(image->size, sizeof(fake_flash_region),
 		"The flash size is correct");
-	memcpy(fake_flash_region, data, data_size);
+	memcpy(fake_flash_region, image->data, image->size);
 	return VB2_SUCCESS;
 }