[flashrom] [PATCH] eliminate magic numbers indicating maximum column sizes in print_supported_chipsets and print_supported_boards_helper
Stefan Tauner
stefan.tauner at student.tuwien.ac.at
Tue May 24 02:21:21 CEST 2011
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 at 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[];
--
1.7.1
More information about the flashrom
mailing list