[flashrom] [PATCH 3/3] an obvious but dubios refactor opportunity

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Mon Aug 15 06:43:38 CEST 2011


it probably makes sense, even if the code savings are minimal, because of
readability. i split it out so that you can decide. as i explained in the previous
patch i hope it wont stay long anyway ;)

Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
---
 layout.c |   50 +++++++++++++++++++-------------------------------
 1 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/layout.c b/layout.c
index 8283e36..fe29852 100644
--- a/layout.c
+++ b/layout.c
@@ -240,6 +240,19 @@ static struct rom_entry *get_next_included_romentry(unsigned int start)
 	return best_entry;
 }
 
+static void copy_old_content(struct flashchip *flash, int oldcontents_valid, uint8_t *oldcontents, uint8_t *newcontents, unsigned int start, unsigned int size)
+{
+	if (!oldcontents_valid) {
+		/* oldcontents is a zero-filled buffer. By reading into
+		 * oldcontents, we avoid a rewrite of identical regions even if
+		 * an initial full chip read didn't happen. */
+		msg_gdbg2("Read a chunk starting from 0x%06x (len=0x%06x).\n",
+			  start, size);
+		flash->read(flash, oldcontents + start, start, size);
+	}
+	memcpy(newcontents + start, oldcontents + start, size);
+}
+
 /**
  * Modify @newcontents so that it contains the data that should be on the chip
  * eventually. In the case the user wants to update only parts of it, copy
@@ -251,7 +264,6 @@ int build_new_image(struct flashchip *flash, int oldcontents_valid, uint8_t *old
 	unsigned int start = 0;
 	struct rom_entry *entry;
 	unsigned int size = flash->total_size * 1024;
-	unsigned int copy_size = 0;
 
 	/* If no layout file was specified or the layout file was empty, assume
 	 * that the user wants to flash the complete new image.
@@ -265,38 +277,14 @@ int build_new_image(struct flashchip *flash, int oldcontents_valid, uint8_t *old
 		entry = get_next_included_romentry(start);
 		/* No more romentries for remaining region? */
 		if (entry == NULL) {
-			copy_size = size - start;
-			if (!oldcontents_valid) {
-				/* oldcontents is a zero-filled buffer. By
-				 * reading into oldcontents, we avoid a rewrite
-				 * of identical regions even if an initial full
-				 * chip read didn't happen. */
-				msg_gdbg("reading last old chunk 0x%06x (0x%06x)\n",
-				     start, copy_size);
-				flash->read(flash, oldcontents + start,
-					    start,
-					    copy_size);
-			}
-			memcpy(newcontents + start, oldcontents + start,
-			       copy_size);
+			copy_old_content(flash, oldcontents_valid, oldcontents,
+					 newcontents, start, size - start);
 			break;
 		}
-		if (entry->start > start) {
-			copy_size = entry->start - start;
-			if (!oldcontents_valid) {
-				/* oldcontents is a zero-filled buffer. By
-				 * reading into oldcontents, we avoid a rewrite
-				 * of identical regions even if an initial full
-				 * chip read didn't happen. */
-				msg_gdbg("reading some chunk 0x%06x (0x%06x)\n",
-				     start, copy_size);
-				flash->read(flash, oldcontents + start,
-					    start,
-					    copy_size);
-			}
-			memcpy(newcontents + start, oldcontents + start,
-			       copy_size);
-		}
+		if (entry->start > start)
+			copy_old_content(flash, oldcontents_valid, oldcontents,
+					 newcontents, start,
+					 entry->start - start);
 		/* Skip to location after current romentry. */
 		start = entry->end + 1;
 		/* Catch overflow. */
-- 
1.7.1





More information about the flashrom mailing list