[PATCH] eliminate magic numbers indicating maximum column sizes in print_supported_chipsets and print_supported_boards_helper
without this the magic numbers need to be kept in sync with the maximum length of the strings printed in the corresponding column. if not an overflow and a nasty ' '-storm occurs on executing flashrom -L.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at ---
after a short discussion with carldani i have removed the assert stuff and do some inferior array checking in selfcheck. we only have _extern_ references to the tables to sizeof does not work. we could add a "getter" for the sizes of the array or bring the tables into view to improve this... but i dont think it's worth it.
flashrom.c | 22 +++++++++++++ print.c | 96 ++++++++++++++++++++++++++++++++++++++------------------- programmer.h | 8 ++-- 3 files changed, 90 insertions(+), 36 deletions(-)
diff --git a/flashrom.c b/flashrom.c index 46c9ecd..821d745 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1709,6 +1709,28 @@ int selfcheck(void) msg_gerr("Programmer table miscompilation!\n"); ret = 1; } + /* It would be favorable if we could also check for correct terminaion + * of the follwing arrays, but we don't know their size in here... */ + if (flashchips == NULL || flashchips[0].vendor == NULL) { + msg_gerr("Flashchips table miscompilation!\n"); + ret = 1; + } + if (chipset_enables == NULL || chipset_enables[0].vendor_name == NULL) { + msg_gerr("Chipset enables table miscompilation!\n"); + ret = 1; + } + if (board_pciid_enables == NULL || board_pciid_enables[0].vendor_name == NULL) { + msg_gerr("Board enables table miscompilation!\n"); + ret = 1; + } + if (boards_known == NULL || boards_known[0].vendor == NULL) { + msg_gerr("Known boards table miscompilation!\n"); + ret = 1; + } + if (laptops_known == NULL || laptops_known[0].vendor == NULL) { + msg_gerr("Known laptops table miscompilation!\n"); + ret = 1; + } for (flash = flashchips; flash && flash->name; flash++) if (selfcheck_eraseblocks(flash)) ret = 1; diff --git a/print.c b/print.c index e84a417..b0f8247 100644 --- a/print.c +++ b/print.c @@ -77,7 +77,8 @@ static int digits(int n) static void print_supported_chips(void) { int okcol = 0, pos = 0, i, chipcount = 0; - int maxchiplen = 0, maxvendorlen = 0; + int maxvendorlen = strlen("Vendor") + 1; + int maxchiplen = strlen("Device") + 1; const struct flashchip *f;
for (f = flashchips; f->name != NULL; f++) { @@ -158,63 +159,94 @@ static void print_supported_chips(void) #if CONFIG_INTERNAL == 1 static void print_supported_chipsets(void) { - int i, j, chipsetcount = 0; + int i, chipsetcount = 0; const struct penable *c = chipset_enables; + int maxvendorlen = strlen("Vendor") + 1; + int maxchipsetlen = strlen("Chipset") + 1;
- for (i = 0; c[i].vendor_name != NULL; i++) + for (c = chipset_enables; c->vendor_name != NULL; c++) { chipsetcount++; + maxvendorlen = max(maxvendorlen, strlen(c->vendor_name)); + maxchipsetlen = max(maxchipsetlen, strlen(c->device_name)); + } + maxvendorlen++; + maxchipsetlen++; + + printf("\nSupported chipsets (total: %d):\n\n", chipsetcount); + + printf("Vendor"); + for (i = strlen("Vendor"); i < maxvendorlen; i++) + printf(" "); + + printf("Chipset"); + for (i = strlen("Chipset"); i < maxchipsetlen; i++) + printf(" ");
- printf("\nSupported chipsets (total: %d):\n\nVendor: " - "Chipset: PCI IDs:\n\n", chipsetcount); + printf("PCI IDs State\n\n");
- for (i = 0; c[i].vendor_name != NULL; i++) { - printf("%s", c[i].vendor_name); - for (j = 0; j < 25 - strlen(c[i].vendor_name); j++) + for (c = chipset_enables; c->vendor_name != NULL; c++) { + printf("%s", c->vendor_name); + for (i = 0; i < maxvendorlen - strlen(c->vendor_name); i++) printf(" "); - printf("%s", c[i].device_name); - for (j = 0; j < 25 - strlen(c[i].device_name); j++) + printf("%s", c->device_name); + for (i = 0; i < maxchipsetlen - strlen(c->device_name); i++) printf(" "); - printf("%04x:%04x%s\n", c[i].vendor_id, c[i].device_id, - (c[i].status == OK) ? "" : " (untested)"); + printf("%04x:%04x%s\n", c->vendor_id, c->device_id, + (c->status == OK) ? "" : " (untested)"); } }
static void print_supported_boards_helper(const struct board_info *boards, const char *devicetype) { - int i, j, boardcount_good = 0, boardcount_bad = 0; - const struct board_pciid_enable *b = board_pciid_enables; - - for (i = 0; boards[i].vendor != NULL; i++) { - if (boards[i].working) + int i, boardcount_good = 0, boardcount_bad = 0; + const struct board_pciid_enable *e = board_pciid_enables; + const struct board_info *b = boards; + int maxvendorlen = strlen("Vendor") + 1; + int maxboardlen = strlen("Board") + 1; + + for (b = boards; b->vendor != NULL; b++) { + maxvendorlen = max(maxvendorlen, strlen(b->vendor)); + maxboardlen = max(maxboardlen, strlen(b->name)); + if (b->working) boardcount_good++; else boardcount_bad++; } + maxvendorlen++; + maxboardlen++; + + printf("\nKnown %s (good: %d, bad: %d):\n\n", + devicetype, boardcount_good, boardcount_bad); + + printf("Vendor"); + for (i = strlen("Vendor"); i < maxvendorlen; i++) + printf(" "); + + printf("Board"); + for (i = strlen("Board"); i < maxboardlen; i++) + printf(" ");
- printf("\nKnown %s (good: %d, bad: %d):" - "\n\nVendor: Board: " - "Status: Required option:" - "\n\n", devicetype, boardcount_good, boardcount_bad); + printf("Status Required option\n\n");
- for (i = 0; boards[i].vendor != NULL; i++) { - printf("%s", boards[i].vendor); - for (j = 0; j < 25 - strlen(boards[i].vendor); j++) + for (b = boards; b->vendor != NULL; b++) { + printf("%s", b->vendor); + for (i = 0; i < maxvendorlen - strlen(b->vendor); i++) printf(" "); - printf("%s", boards[i].name); - for (j = 0; j < 28 - strlen(boards[i].name); j++) + printf("%s", b->name); + for (i = 0; i < maxboardlen - strlen(b->name); i++) printf(" "); - printf((boards[i].working) ? "OK " : "BAD "); + printf((b->working) ? "OK " : "BAD ");
- for (j = 0; b[j].vendor_name != NULL; j++) { - if (strcmp(b[j].vendor_name, boards[i].vendor) - || strcmp(b[j].board_name, boards[i].name)) + for (e = board_pciid_enables; e->vendor_name != NULL; e++) { + if (strcmp(e->vendor_name, b->vendor) + || strcmp(e->board_name, b->name)) continue; - if (b[j].lb_vendor == NULL) + if (e->lb_vendor == NULL) printf("(autodetected)"); else - printf("-m %s:%s", b[j].lb_vendor, - b[j].lb_part); + printf("-m %s:%s", e->lb_vendor, + e->lb_part); } printf("\n"); } diff --git a/programmer.h b/programmer.h index b68aa88..e1147f1 100644 --- a/programmer.h +++ b/programmer.h @@ -145,7 +145,7 @@ struct bitbang_spi_master { struct penable { uint16_t vendor_id; uint16_t device_id; - int status; + int status; /* OK=0 and NT=1 are defines only. Beware! */ const char *vendor_name; const char *device_name; int (*doit) (struct pci_dev *dev, const char *name); @@ -174,10 +174,10 @@ struct board_pciid_enable { uint16_t second_card_vendor; uint16_t second_card_device;
- /* Pattern to match DMI entries */ + /* Pattern to match DMI entries. May be NULL. */ const char *dmi_pattern;
- /* The vendor / part name from the coreboot table. */ + /* The vendor / part name from the coreboot table. May be NULL. */ const char *lb_vendor; const char *lb_part;
@@ -188,7 +188,7 @@ struct board_pciid_enable {
int max_rom_decode_parallel; int status; - int (*enable) (void); + int (*enable) (void); /* May be NULL. */ };
extern const struct board_pciid_enable board_pciid_enables[];
-
Stefan Tauner