Author: stefanct Date: Tue May 27 00:05:31 2014 New Revision: 1799 URL: http://flashrom.org/trac/flashrom/changeset/1799
Log: Fix selfcheck of various arrays.
Stefan Reinauer has reported ridiculous NULL checks for arrays in our self_check function found by Coverity (CID1130005). This patch removes the useless checks but keeps and fixes the one responsible for the flashchips array by exporting the array size in a new constant.
Signed-off-by: Stefan Tauner stefan.tauner@alumni.tuwien.ac.at Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Modified: trunk/flash.h trunk/flashchips.c trunk/flashrom.c
Modified: trunk/flash.h ============================================================================== --- trunk/flash.h Mon May 26 02:36:24 2014 (r1798) +++ trunk/flash.h Tue May 27 00:05:31 2014 (r1799) @@ -225,6 +225,7 @@ #define TIMING_ZERO -2
extern const struct flashchip flashchips[]; +extern const unsigned int flashchips_size;
void chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr); void chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr);
Modified: trunk/flashchips.c ============================================================================== --- trunk/flashchips.c Mon May 26 02:36:24 2014 (r1798) +++ trunk/flashchips.c Tue May 27 00:05:31 2014 (r1799) @@ -13424,5 +13424,7 @@ .write = NULL, },
- { NULL } + {0} }; + +const unsigned int flashchips_size = ARRAY_SIZE(flashchips);
Modified: trunk/flashrom.c ============================================================================== --- trunk/flashrom.c Mon May 26 02:36:24 2014 (r1798) +++ trunk/flashrom.c Tue May 27 00:05:31 2014 (r1799) @@ -1269,10 +1269,7 @@ 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. - */ +/* 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; @@ -1694,8 +1691,7 @@
int selfcheck(void) { - const struct flashchip *chip; - int i; + unsigned int i; int ret = 0;
/* Safety check. Instead of aborting after the first error, check @@ -1748,37 +1744,32 @@ 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) { + + /* 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 export the size explicitly to work around this and to be able to implement the + * checks below. */ + if (flashchips_size <= 1 || flashchips[flashchips_size-1].name != NULL) { msg_gerr("Flashchips table miscompilation!\n"); ret = 1; + } else { + for (i = 0; i < flashchips_size - 1; i++) { + const struct flashchip *chip = &flashchips[i]; + if (chip->vendor == NULL || chip->name == NULL || chip->bustype == BUS_NONE) { + ret = 1; + msg_gerr("ERROR: Some field of flash chip #%d (%s) is misconfigured.\n" + "Please report a bug at flashrom@flashrom.org\n", i, + chip->name == NULL ? "unnamed" : chip->name); + } + if (selfcheck_eraseblocks(chip)) { + 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"); - 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 similar sanity checks for other arrays where deemed necessary. */ return ret; }