[M] Change in flashrom[main]: cli: Remove array of flash contexts, use one active context instead

[Anastasia Klimchuk (Code Review)]

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/87533?usp=email )

Change subject: cli: Remove array of flash contexts, use one active context instead
......................................................................

cli: Remove array of flash contexts, use one active context instead

The only purpose of array of 8 flash contexts was to facilitate
probing of multiple chips. Probing is now done inside libflashrom,
and cli as a client of libflashrom only need to have one active
context.

In addition to array of 8, cli also had one more flash context,
`fill_flash` which is also now replaced by the same, one active
context.

Another detail is that array of 8 was effectively a limit of how
many mathing chips could be found. While 8 seemed a lot at the time
of initial implementation, at the moment we have an example of
6 matches already.
(see `./flashrom -p dummy:emulate=MX25L6436`)

Change-Id: Ia4284ae7aaa43fe59f0d3f57314ebc5cc93d2d9b
Signed-off-by: Anastasia Klimchuk <[e-mail]>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/87533
Reviewed-by: Peter Marheine <[e-mail]>
Tested-by: build bot (Jenkins) <[e-mail]>
---
M cli_classic.c
1 file changed, 37 insertions(+), 43 deletions(-)

Approvals:
  Peter Marheine: Looks good to me, approved
  build bot (Jenkins): Verified




diff --git a/cli_classic.c b/cli_classic.c
index f465785..8b6474b 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -1046,9 +1046,7 @@
 int main(int argc, char *argv[])
 {
 	const struct flashchip *chip = NULL;
-	/* Probe for up to eight flash chips. */
-	struct flashctx flashes[8] = {{0}};
-	struct flashctx *fill_flash;
+	struct flashctx context = {0}; /* holds the active detected chip and other info */
 	char *tempstr = NULL;
 	int i, j;
 	int ret = 0;
@@ -1210,7 +1208,7 @@
 	msg_pdbg("The following protocols are supported: %s.\n", tempstr ? tempstr : "?");
 	free(tempstr);
 
-	all_matched_count = flashrom_flash_probe_v2(&flashes[0], &all_matched_names,
+	all_matched_count = flashrom_flash_probe_v2(&context, &all_matched_names,
                                 NULL, options.chip_to_probe);
 	if (all_matched_count == -1) {
 		/* -1 is the ret code which means "something went wrong".
@@ -1223,7 +1221,7 @@
 
 	if (all_matched_count > 1) {
 		msg_cinfo("Multiple flash chip definitions match the detected chip(s): \"%s\"",
-			  flashes[0].chip->name);
+			  context.chip->name);
 		for (int ind = 1; ind < all_matched_count; ind++)
 			msg_cinfo(", \"%s\"", all_matched_names[ind]);
 		msg_cinfo("\nPlease specify which chip definition to use with the -c <chipname> option.\n");
@@ -1258,7 +1256,7 @@
 			int startchip = -1;
 			for (j = 0; j < registered_master_count; j++) {
 				mst = ®istered_masters[j];
-				startchip = probe_flash(mst, 0, &flashes[0], 1, options.chip_to_probe);
+				startchip = probe_flash(mst, 0, &context, 1, options.chip_to_probe);
 				if (startchip != -1)
 					break;
 			}
@@ -1269,33 +1267,31 @@
 				goto out_shutdown;
 			}
 			msg_cinfo("Please note that forced reads most likely contain garbage.\n");
-			flashrom_flag_set(&flashes[0], FLASHROM_FLAG_FORCE, options.force);
-			ret = do_read(&flashes[0], options.filename);
-			free(flashes[0].chip);
+			flashrom_flag_set(&context, FLASHROM_FLAG_FORCE, options.force);
+			ret = do_read(&context, options.filename);
+			free(context.chip);
 			goto out_shutdown;
 		}
 		ret = 1;
 		goto out_shutdown;
 	} else if (!options.chip_to_probe) {
 		/* repeat for convenience when looking at foreign logs */
-		tempstr = flashbuses_to_text(flashes[0].chip->bustype);
+		tempstr = flashbuses_to_text(context.chip->bustype);
 		msg_gdbg("Found %s flash chip \"%s\" (%d kB, %s).\n",
-			 flashes[0].chip->vendor, flashes[0].chip->name, flashes[0].chip->total_size,
+			 context.chip->vendor, context.chip->name, context.chip->total_size,
 			 tempstr ? tempstr : "?");
 		free(tempstr);
 	}
 
-	fill_flash = &flashes[0];
-
 	struct cli_progress cli_progress = {0};
 	if (options.show_progress)
-		flashrom_set_progress_callback_v2(fill_flash, &flashrom_progress_cb, &cli_progress);
+		flashrom_set_progress_callback_v2(&context, &flashrom_progress_cb, &cli_progress);
 
-	print_chip_support_status(fill_flash->chip);
+	print_chip_support_status(context.chip);
 
-	unsigned int limitexceeded = count_max_decode_exceedings(fill_flash, &max_rom_decode);
+	unsigned int limitexceeded = count_max_decode_exceedings(&context, &max_rom_decode);
 	if (limitexceeded > 0 && !options.force) {
-		enum chipbustype commonbuses = fill_flash->mst->buses_supported & fill_flash->chip->bustype;
+		enum chipbustype commonbuses = context.mst->buses_supported & context.chip->bustype;
 
 		/* Sometimes chip and programmer have more than one bus in common,
 		 * and the limit is not exceeded on all buses. Tell the user. */
@@ -1393,10 +1389,10 @@
 	}
 
 	if (options.flash_name) {
-		if (fill_flash->chip->vendor && fill_flash->chip->name) {
+		if (context.chip->vendor && context.chip->name) {
 			printf("vendor=\"%s\" name=\"%s\"\n",
-				fill_flash->chip->vendor,
-				fill_flash->chip->name);
+				context.chip->vendor,
+				context.chip->name);
 		} else {
 			ret = -1;
 		}
@@ -1404,7 +1400,7 @@
 	}
 
 	if (options.flash_size) {
-		printf("%zu\n", flashrom_flash_getsize(fill_flash));
+		printf("%zu\n", flashrom_flash_getsize(&context));
 		goto out_shutdown;
 	}
 
@@ -1413,10 +1409,10 @@
 			msg_ginfo("Invalid input of sacrifice ratio, valid 0-50. Fallback to default value 0.\n");
 			options.sacrifice_ratio = 0;
 		}
-		fill_flash->sacrifice_ratio = options.sacrifice_ratio;
+		context.sacrifice_ratio = options.sacrifice_ratio;
 	}
 
-	if (options.ifd && (flashrom_layout_read_from_ifd(&options.layout, fill_flash, NULL, 0) ||
+	if (options.ifd && (flashrom_layout_read_from_ifd(&options.layout, &context, NULL, 0) ||
 			   process_include_args(options.layout, options.include_args))) {
 		ret = 1;
 		goto out_shutdown;
@@ -1441,20 +1437,20 @@
 			goto out_shutdown;
 		}
 
-		if (flashrom_layout_read_fmap_from_buffer(&options.layout, fill_flash, fmapfile_buffer, fmapfile_size) ||
+		if (flashrom_layout_read_fmap_from_buffer(&options.layout, &context, fmapfile_buffer, fmapfile_size) ||
 		    process_include_args(options.layout, options.include_args)) {
 			ret = 1;
 			free(fmapfile_buffer);
 			goto out_shutdown;
 		}
 		free(fmapfile_buffer);
-	} else if (options.fmap && (flashrom_layout_read_fmap_from_rom(&options.layout, fill_flash, 0,
-				flashrom_flash_getsize(fill_flash)) ||
+	} else if (options.fmap && (flashrom_layout_read_fmap_from_rom(&options.layout, &context, 0,
+				flashrom_flash_getsize(&context)) ||
 				process_include_args(options.layout, options.include_args))) {
 		ret = 1;
 		goto out_shutdown;
 	}
-	flashrom_layout_set(fill_flash, options.layout);
+	flashrom_layout_set(&context, options.layout);
 
 	if (any_wp_op) {
 		if (options.set_wp_region && options.wp_region) {
@@ -1472,7 +1468,7 @@
 			options.set_wp_range = true;
 		}
 		ret = wp_cli(
-			fill_flash,
+			&context,
 			options.enable_wp,
 			options.disable_wp,
 			options.print_wp_status,
@@ -1485,24 +1481,24 @@
 			goto out_release;
 	}
 
-	flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, options.force);
+	flashrom_flag_set(&context, FLASHROM_FLAG_FORCE, options.force);
 #if CONFIG_INTERNAL == 1
-	flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE_BOARDMISMATCH, force_boardmismatch);
+	flashrom_flag_set(&context, FLASHROM_FLAG_FORCE_BOARDMISMATCH, force_boardmismatch);
 #endif
-	flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_AFTER_WRITE, !options.dont_verify_it);
-	flashrom_flag_set(fill_flash, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, !options.dont_verify_all);
+	flashrom_flag_set(&context, FLASHROM_FLAG_VERIFY_AFTER_WRITE, !options.dont_verify_it);
+	flashrom_flag_set(&context, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, !options.dont_verify_all);
 
 	/* FIXME: We should issue an unconditional chip reset here. This can be
 	 * done once we have a .reset function in struct flashchip.
 	 * Give the chip time to settle.
 	 */
-	programmer_delay(fill_flash, 100000);
+	programmer_delay(&context, 100000);
 	if (options.read_it)
-		ret = do_read(fill_flash, options.filename);
+		ret = do_read(&context, options.filename);
 	else if (options.extract_it)
-		ret = do_extract(fill_flash);
+		ret = do_extract(&context);
 	else if (options.erase_it) {
-		ret = flashrom_flash_erase(fill_flash);
+		ret = flashrom_flash_erase(&context);
 		/*
 		 * FIXME: Do we really want the scary warning if erase failed?
 		 * After all, after erase the chip is either blank or partially
@@ -1514,13 +1510,13 @@
 			emergency_help_message();
 	}
 	else if (options.write_it)
-		ret = do_write(fill_flash, options.filename, options.referencefile);
+		ret = do_write(&context, options.filename, options.referencefile);
 	else if (options.verify_it)
-		ret = do_verify(fill_flash, options.filename);
+		ret = do_verify(&context, options.filename);
 
 #if CONFIG_RPMC_ENABLED == 1
 	if (any_rpmc_op && ret == 0) {
-		ret = rpmc_cli(fill_flash,
+		ret = rpmc_cli(&context,
 			       options.rpmc_root_key_file,
 			       options.rpmc_key_data,
 			       options.rpmc_counter_address,
@@ -1538,10 +1534,8 @@
 out_shutdown:
 	flashrom_programmer_shutdown(NULL);
 out:
-	for (int ind = 0; ind < all_matched_count; ind++) {
-		flashrom_layout_release(flashes[ind].default_layout);
-		free(flashes[ind].chip);
-	}
+	flashrom_layout_release(context.default_layout);
+	free(context.chip);
 	flashrom_data_free(all_matched_names);
 
 	free_options(&options);

-- 
To view, visit https://review.coreboot.org/c/flashrom/+/87533?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Ia4284ae7aaa43fe59f0d3f57314ebc5cc93d2d9b
Gerrit-Change-Number: 87533
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <[e-mail]>
Gerrit-Reviewer: Anastasia Klimchuk <[e-mail]>
Gerrit-Reviewer: Peter Marheine <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>