From 96658be4073aaa55101b17e95c7b704a2cc7c83a Mon Sep 17 00:00:00 2001 From: Stefan Tauner Date: Mon, 26 May 2014 22:05:31 +0000 Subject: 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. Corresponding to flashrom svn r1799. Signed-off-by: Stefan Tauner Acked-by: Carl-Daniel Hailfinger --- flashrom.c | 57 ++++++++++++++++++++++++--------------------------------- 1 file changed, 24 insertions(+), 33 deletions(-) (limited to 'flashrom.c') diff --git a/flashrom.c b/flashrom.c index a2851afbc..10e375ad8 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1269,10 +1269,7 @@ 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. - */ +/* 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 @@ void print_banner(void) 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 @@ 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) { + + /* 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; } -- cgit v1.2.3