>From 8b53a38afed153d8252e2f77a70b9ff2da5090ff Mon Sep 17 00:00:00 2001
From: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>
Date: Thu, 8 May 2014 18:54:17 +0200
Subject: [PATCH] Move selfcheck of array 'flashchips' into flashchips.c.

This allows to check for correct size unlike other options outside the file.
Also, this fixes CID1130005 as reported by Stefan Reinauer as well by
removing the other futile NULL checks and explaining the issues better.

Signed-off-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>
---
 flash.h      |   1 +
 flashchips.c |  85 ++++++++++++++++++++++++++++++++++++++++++++++-
 flashrom.c   | 106 +++++------------------------------------------------------
 3 files changed, 93 insertions(+), 99 deletions(-)

diff --git a/flash.h b/flash.h
index 59f7cd4..90f1449 100644
--- a/flash.h
+++ b/flash.h
@@ -224,6 +224,7 @@ struct flashctx {
 #define TIMING_ZERO	-2
 
 extern const struct flashchip flashchips[];
+int selfcheck_flashchips(void);
 
 void chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr);
 void chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr);
diff --git a/flashchips.c b/flashchips.c
index 8059374..4de1b6e 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -13362,5 +13362,88 @@ const struct flashchip flashchips[] = {
 		.write		= NULL,
 	},
 
-	{ NULL 	}
+	{0}
 };
+
+/* Even if an error is found, the function will keep going and check the rest. */
+static int selfcheck_eraseblocks(const struct flashchip *chip)
+{
+	int i, j, k;
+	int ret = 0;
+
+	for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
+		unsigned int done = 0;
+		struct block_eraser eraser = chip->block_erasers[k];
+
+		for (i = 0; i < NUM_ERASEREGIONS; i++) {
+			/* Blocks with zero size are bugs in flashchips.c. */
+			if (eraser.eraseblocks[i].count &&
+			    !eraser.eraseblocks[i].size) {
+				msg_gerr("ERROR: Flash chip %s erase function "
+					"%i region %i has size 0. Please report"
+					" a bug at flashrom@flashrom.org\n",
+					chip->name, k, i);
+				ret = 1;
+			}
+			/* Blocks with zero count are bugs in flashchips.c. */
+			if (!eraser.eraseblocks[i].count &&
+			    eraser.eraseblocks[i].size) {
+				msg_gerr("ERROR: Flash chip %s erase function "
+					"%i region %i has count 0. Please report"
+					" a bug at flashrom@flashrom.org\n",
+					chip->name, k, i);
+				ret = 1;
+			}
+			done += eraser.eraseblocks[i].count *
+				eraser.eraseblocks[i].size;
+		}
+		/* Empty eraseblock definition with erase function.  */
+		if (!done && eraser.block_erase)
+			msg_gspew("Strange: Empty eraseblock definition with "
+				  "non-empty erase function. Not an error.\n");
+		if (!done)
+			continue;
+		if (done != chip->total_size * 1024) {
+			msg_gerr("ERROR: Flash chip %s erase function %i "
+				"region walking resulted in 0x%06x bytes total,"
+				" expected 0x%06x bytes. Please report a bug at"
+				" flashrom@flashrom.org\n", chip->name, k,
+				done, chip->total_size * 1024);
+			ret = 1;
+		}
+		if (!eraser.block_erase)
+			continue;
+		/* Check if there are identical erase functions for different
+		 * layouts. That would imply "magic" erase functions. The
+		 * easiest way to check this is with function pointers.
+		 */
+		for (j = k + 1; j < NUM_ERASEFUNCTIONS; j++) {
+			if (eraser.block_erase ==
+			    chip->block_erasers[j].block_erase) {
+				msg_gerr("ERROR: Flash chip %s erase function "
+					"%i and %i are identical. Please report"
+					" a bug at flashrom@flashrom.org\n",
+					chip->name, k, j);
+				ret = 1;
+			}
+		}
+	}
+	return ret;
+}
+
+int selfcheck_flashchips(void)
+{
+	if (ARRAY_SIZE(flashchips) <= 1 || flashchips[0].vendor == NULL) {
+		msg_gerr("Flashchips table miscompilation!\n");
+		return 1;
+	}
+
+	int ret = 0;
+	const struct flashchip *chip;
+	for (chip = flashchips; chip && chip->name; chip++) {
+		if (selfcheck_eraseblocks(chip)) {
+			ret = 1;
+		}
+	}
+	return ret;
+}
diff --git a/flashrom.c b/flashrom.c
index c20461a..e044ca0 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1271,75 +1271,6 @@ out_free:
 	return ret;
 }
 
-/* This function shares a lot of its structure with erase_and_write_flash() and
- * walk_eraseregions().
- * Even if an error is found, the function will keep going and check the rest.
- */
-static int selfcheck_eraseblocks(const struct flashchip *chip)
-{
-	int i, j, k;
-	int ret = 0;
-
-	for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
-		unsigned int done = 0;
-		struct block_eraser eraser = chip->block_erasers[k];
-
-		for (i = 0; i < NUM_ERASEREGIONS; i++) {
-			/* Blocks with zero size are bugs in flashchips.c. */
-			if (eraser.eraseblocks[i].count &&
-			    !eraser.eraseblocks[i].size) {
-				msg_gerr("ERROR: Flash chip %s erase function "
-					"%i region %i has size 0. Please report"
-					" a bug at flashrom@flashrom.org\n",
-					chip->name, k, i);
-				ret = 1;
-			}
-			/* Blocks with zero count are bugs in flashchips.c. */
-			if (!eraser.eraseblocks[i].count &&
-			    eraser.eraseblocks[i].size) {
-				msg_gerr("ERROR: Flash chip %s erase function "
-					"%i region %i has count 0. Please report"
-					" a bug at flashrom@flashrom.org\n",
-					chip->name, k, i);
-				ret = 1;
-			}
-			done += eraser.eraseblocks[i].count *
-				eraser.eraseblocks[i].size;
-		}
-		/* Empty eraseblock definition with erase function.  */
-		if (!done && eraser.block_erase)
-			msg_gspew("Strange: Empty eraseblock definition with "
-				  "non-empty erase function. Not an error.\n");
-		if (!done)
-			continue;
-		if (done != chip->total_size * 1024) {
-			msg_gerr("ERROR: Flash chip %s erase function %i "
-				"region walking resulted in 0x%06x bytes total,"
-				" expected 0x%06x bytes. Please report a bug at"
-				" flashrom@flashrom.org\n", chip->name, k,
-				done, chip->total_size * 1024);
-			ret = 1;
-		}
-		if (!eraser.block_erase)
-			continue;
-		/* Check if there are identical erase functions for different
-		 * layouts. That would imply "magic" erase functions. The
-		 * easiest way to check this is with function pointers.
-		 */
-		for (j = k + 1; j < NUM_ERASEFUNCTIONS; j++) {
-			if (eraser.block_erase ==
-			    chip->block_erasers[j].block_erase) {
-				msg_gerr("ERROR: Flash chip %s erase function "
-					"%i and %i are identical. Please report"
-					" a bug at flashrom@flashrom.org\n",
-					chip->name, k, j);
-				ret = 1;
-			}
-		}
-	}
-	return ret;
-}
-
 static int erase_and_write_block_helper(struct flashctx *flash,
 					unsigned int start, unsigned int len,
 					uint8_t *curcontents,
@@ -1697,7 +1628,6 @@ void print_banner(void)
 
 int selfcheck(void)
 {
-	const struct flashchip *chip;
 	int i;
 	int ret = 0;
 
@@ -1751,37 +1681,17 @@ int selfcheck(void)
 			ret = 1;
 		}
 	}
-	/* It would be favorable if we could also check for correct termination
-	 * of the following arrays, but we don't know their sizes in here...
-	 * For 'flashchips' we check the first element to be non-null. In the
-	 * other cases there exist use cases where the first element can be
-	 * null. */
-	if (flashchips == NULL || flashchips[0].vendor == NULL) {
-		msg_gerr("Flashchips table miscompilation!\n");
-		ret = 1;
-	}
-	for (chip = flashchips; chip && chip->name; chip++)
-		if (selfcheck_eraseblocks(chip))
-			ret = 1;
 
-#if CONFIG_INTERNAL == 1
-	if (chipset_enables == NULL) {
-		msg_gerr("Chipset enables table does not exist!\n");
+	/* It would be favorable if we could check for the correct layout (especially termination) of various
+	 * constant arrays: flashchips, chipset_enables, board_matches, boards_known, laptops_known.
+	 * They are all defined as externs in this compilation unit so we don't know their sizes which vary
+	 * depending on compiler flags, e.g. the target architecture, and can sometimes be 0.
+	 * For 'flashchips' we implement a dedicated check function inside flashchips.c to work around this. */
+	if (selfcheck_flashchips() != 0) {
 		ret = 1;
 	}
-	if (board_matches == NULL) {
-		msg_gerr("Board enables table does not exist!\n");
-		ret = 1;
-	}
-	if (boards_known == NULL) {
-		msg_gerr("Known boards table does not exist!\n");
-		ret = 1;
-	}
-	if (laptops_known == NULL) {
-		msg_gerr("Known laptops table does not exist!\n");
-		ret = 1;
-	}
-#endif
+
+	/* TODO: implement and call self-checking functions in other files where deemed necessary. */
 	return ret;
 }
 
-- 
Kind regards, Stefan Tauner

