Change in flashrom[master]: flashrom.c: wrap chip->read() with a helper

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/64800 )


Change subject: flashrom.c: wrap chip->read() with a helper
......................................................................

flashrom.c: wrap chip->read() with a helper

Change-Id: I7ebb45fba241c8d6041032df5f22fd3b03fca9f5
Signed-off-by: Edward O'Callaghan <[e-mail]>
---
M flashrom.c
1 file changed, 38 insertions(+), 9 deletions(-)



  git pull ssh://review.coreboot.org:29418/flashrom refs/changes/00/64800/1

diff --git a/flashrom.c b/flashrom.c
index 8d6b3db..339e744 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -330,6 +330,35 @@
 	return extract_param(&programmer_param, param_name, ",");
 }
 
+/*
+ * @brief Wrapper for flash->read() with additional high-level policy.
+ *
+ * @flash       flash chip
+ * @buf         buffer to store data in
+ * @start       start address
+ * @len         number of bytes to read
+ * @return	0 on success,
+ *		-1 if any read fails.
+ *
+ * This wrapper simplifies most cases when the flash chip needs to be read
+ * since policy decisions such as non-fatal error handling is centralized.
+ */
+static int read_flash(struct flashctx *flash, uint8_t *buf,
+		      unsigned int start, unsigned int len)
+{
+	int ret;
+
+	if (!flash || !flash->chip->read)
+		return -1;
+
+	ret = flash->chip->read(flash, buf, start, len);
+	if (ret) {
+		msg_gdbg("failed to read 0x%x-0x%x\n", start, start + len - 1);
+	}
+
+	return ret;
+}
+
 static int check_block_eraser(const struct flashctx *flash, int k, int log)
 {
 	struct block_eraser eraser = flash->chip->block_erasers[k];
@@ -434,7 +463,7 @@
 		return -1;
 	}
 
-	int ret = flash->chip->read(flash, readbuf, start, len);
+	int ret = read_flash(flash, readbuf, start, len);
 	if (ret) {
 		msg_gerr("Verification impossible because read failed "
 			 "at 0x%x (len 0x%x)\n", start, len);
@@ -1064,7 +1093,7 @@
 		const chipoff_t region_start	= entry->start;
 		const chipsize_t region_len	= entry->end - entry->start + 1;
 
-		if (flashctx->chip->read(flashctx, buffer + region_start, region_start, region_len))
+		if (read_flash(flashctx, buffer + region_start, region_start, region_len))
 			return 1;
 	}
 	return 0;
@@ -1282,7 +1311,7 @@
 		if (info->region_start > info->erase_start) {
 			const chipoff_t start	= info->erase_start;
 			const chipsize_t len	= info->region_start - info->erase_start;
-			if (flashctx->chip->read(flashctx, backup_contents, start, len)) {
+			if (read_flash(flashctx, backup_contents, start, len)) {
 				msg_cerr("Can't read! Aborting.\n");
 				goto _free_ret;
 			}
@@ -1292,7 +1321,7 @@
 			const chipoff_t start     = info->region_end + 1;
 			const chipoff_t rel_start = start - info->erase_start; /* within this erase block */
 			const chipsize_t len      = info->erase_end - info->region_end;
-			if (flashctx->chip->read(flashctx, backup_contents + rel_start, start, len)) {
+			if (read_flash(flashctx, backup_contents + rel_start, start, len)) {
 				msg_cerr("Can't read! Aborting.\n");
 				goto _free_ret;
 			}
@@ -1379,7 +1408,7 @@
 		if (info->region_start > info->erase_start) {
 			const chipoff_t start	= info->erase_start;
 			const chipsize_t len	= info->region_start - info->erase_start;
-			if (flashctx->chip->read(flashctx, newc, start, len)) {
+			if (read_flash(flashctx, newc, start, len)) {
 				msg_cerr("Can't read! Aborting.\n");
 				goto _free_ret;
 			}
@@ -1390,7 +1419,7 @@
 			const chipoff_t start     = info->region_end + 1;
 			const chipoff_t rel_start = start - info->erase_start; /* within this erase block */
 			const chipsize_t len      = info->erase_end - info->region_end;
-			if (flashctx->chip->read(flashctx, newc + rel_start, start, len)) {
+			if (read_flash(flashctx, newc + rel_start, start, len)) {
 				msg_cerr("Can't read! Aborting.\n");
 				goto _free_ret;
 			}
@@ -1490,7 +1519,7 @@
 		const chipoff_t region_start	= entry->start;
 		const chipsize_t region_len	= entry->end - entry->start + 1;
 
-		if (flashctx->chip->read(flashctx, curcontents + region_start, region_start, region_len))
+		if (read_flash(flashctx, curcontents + region_start, region_start, region_len))
 			return 1;
 		if (compare_range(newcontents + region_start, curcontents + region_start,
 				  region_start, region_len))
@@ -1985,7 +2014,7 @@
 		 */
 		msg_cinfo("Reading old flash chip contents... ");
 		if (verify_all) {
-			if (flashctx->chip->read(flashctx, oldcontents, 0, flash_size)) {
+			if (read_flash(flashctx, oldcontents, 0, flash_size)) {
 				msg_cinfo("FAILED.\n");
 				goto _finalize_ret;
 			}
@@ -2005,7 +2034,7 @@
 		if (verify_all) {
 			msg_cerr("Checking if anything has changed.\n");
 			msg_cinfo("Reading current flash chip contents... ");
-			if (!flashctx->chip->read(flashctx, curcontents, 0, flash_size)) {
+			if (!read_flash(flashctx, curcontents, 0, flash_size)) {
 				msg_cinfo("done.\n");
 				if (!memcmp(oldcontents, curcontents, flash_size)) {
 					nonfatal_help_message();

-- 
To view, visit https://review.coreboot.org/c/flashrom/+/64800
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7ebb45fba241c8d6041032df5f22fd3b03fca9f5
Gerrit-Change-Number: 64800
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <[e-mail]>
Gerrit-MessageType: newchange