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 
---
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();