[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