flashrom: rewrite flashbuses_to_text()

The previous implementation had no error handling, as a result the
flashrom could crash if the computer ran out of memory. The new
version returns NULL in such cases.

Also, rewrite lots of `if` conditions to one cycle, store a name of
buses and `enum chipbustype` in an array by using a custom struct.

The caller always expected a non-null value, so change its behavior to
handle a possible null value or use the `?` symbol. As far as `free()`
can handle null pointers, do nothing with such callers.

TEST=ninja test

Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Ticket: https://ticket.coreboot.org/issues/408
Reviewed-on: https://review.coreboot.org/c/flashrom/+/73039
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>

4 files changed, 54 insertions, 23 deletions

diff --git a/cli_classic.c b/cli_classic.c
index 787de67a..e4db9131 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -1011,7 +1011,7 @@ int main(int argc, char *argv[])
 		goto out_shutdown;
 	}
 	tempstr = flashbuses_to_text(get_buses_supported());
-	msg_pdbg("The following protocols are supported: %s.\n", tempstr);
+	msg_pdbg("The following protocols are supported: %s.\n", tempstr ? tempstr : "?");
 	free(tempstr);
 
 	for (j = 0; j < registered_master_count; j++) {
@@ -1082,7 +1082,8 @@ int main(int argc, char *argv[])
 		/* repeat for convenience when looking at foreign logs */
 		tempstr = flashbuses_to_text(flashes[0].chip->bustype);
 		msg_gdbg("Found %s flash chip \"%s\" (%d kB, %s).\n",
-			 flashes[0].chip->vendor, flashes[0].chip->name, flashes[0].chip->total_size, tempstr);
+			 flashes[0].chip->vendor, flashes[0].chip->name, flashes[0].chip->total_size,
+			 tempstr ? tempstr : "?");
 		free(tempstr);
 	}
 
diff --git a/flashrom.c b/flashrom.c
index 3e6e0fb5..e14516f6 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -65,6 +65,17 @@ static struct shutdown_func_data {
  */
 static bool may_register_shutdown = false;
 
+static struct bus_type_info {
+	enum chipbustype type;
+	const char *name;
+} bustypes[] = {
+	{ BUS_PARALLEL, "Parallel, " },
+	{ BUS_LPC, "LPC, " },
+	{ BUS_FWH, "FWH, " },
+	{ BUS_SPI, "SPI, " },
+	{ BUS_PROG, "Programmer-specific, " },
+};
+
 /* Register a function to be executed on programmer shutdown.
  * The advantage over atexit() is that you can supply a void pointer which will
  * be used as parameter to the registered function upon programmer shutdown.
@@ -912,35 +923,44 @@ int map_flash(struct flashctx *flash)
 
 /*
  * Return a string corresponding to the bustype parameter.
- * Memory is obtained with malloc() and must be freed with free() by the caller.
+ * Memory to store the string is allocated. The caller is responsible to free memory.
+ * If there is not enough memory remaining, then NULL is returned.
  */
 char *flashbuses_to_text(enum chipbustype bustype)
 {
-	char *ret = calloc(1, 1);
+	char *ret, *ptr;
+
 	/*
 	 * FIXME: Once all chipsets and flash chips have been updated, NONSPI
 	 * will cease to exist and should be eliminated here as well.
 	 */
-	if (bustype == BUS_NONSPI) {
-		ret = strcat_realloc(ret, "Non-SPI, ");
-	} else {
-		if (bustype & BUS_PARALLEL)
-			ret = strcat_realloc(ret, "Parallel, ");
-		if (bustype & BUS_LPC)
-			ret = strcat_realloc(ret, "LPC, ");
-		if (bustype & BUS_FWH)
-			ret = strcat_realloc(ret, "FWH, ");
-		if (bustype & BUS_SPI)
-			ret = strcat_realloc(ret, "SPI, ");
-		if (bustype & BUS_PROG)
-			ret = strcat_realloc(ret, "Programmer-specific, ");
-		if (bustype == BUS_NONE)
-			ret = strcat_realloc(ret, "None, ");
+	if (bustype == BUS_NONSPI)
+		return strdup("Non-SPI");
+	if (bustype == BUS_NONE)
+		return strdup("None");
+
+	ret = calloc(1, 1);
+	if (!ret)
+		return NULL;
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(bustypes); i++)
+	{
+		if (bustype & bustypes[i].type) {
+			ptr = strcat_realloc(ret, bustypes[i].name);
+			if (!ptr) {
+				free(ret);
+				return NULL;
+			}
+			ret = ptr;
+		}
 	}
+
 	/* Kill last comma. */
 	ret[strlen(ret) - 2] = '\0';
-	ret = realloc(ret, strlen(ret) + 1);
-	return ret;
+	ptr = realloc(ret, strlen(ret) + 1);
+	if (!ptr)
+		free(ret);
+	return ptr;
 }
 
 static int init_default_layout(struct flashctx *flash)
@@ -1165,7 +1185,7 @@ notfound:
 
 	tmp = flashbuses_to_text(flash->chip->bustype);
 	msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found",
-		  flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp);
+		  flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp ? tmp : "?");
 	free(tmp);
 	if (master_uses_physmap(mst))
 		msg_cinfo("mapped at physical address 0x%0*" PRIxPTR ".\n",
diff --git a/print.c b/print.c
index 98d31094..0caa0da7 100644
--- a/print.c
+++ b/print.c
@@ -103,6 +103,10 @@ static int print_supported_chips(void)
 		} while (1);
 
 		s = flashbuses_to_text(chip->bustype);
+		if (s == NULL) {
+			msg_gerr("Out of memory!\n");
+			return 1;
+		}
 		maxtypelen = max(maxtypelen, strlen(s));
 		free(s);
 	}
@@ -265,6 +269,12 @@ static int print_supported_chips(void)
 			msg_ginfo(" ");
 
 		s = flashbuses_to_text(chip->bustype);
+		if (s == NULL) {
+			msg_gerr("Out of memory!\n");
+			free(ven);
+			free(dev);
+			return 1;
+		}
 		msg_ginfo("%s", s);
 		for (i = strlen(s); i < maxtypelen; i++)
 			msg_ginfo(" ");
diff --git a/print_wiki.c b/print_wiki.c
index 2c8c1093..a0cade9c 100644
--- a/print_wiki.c
+++ b/print_wiki.c
@@ -325,7 +325,7 @@ static void print_supported_chips_wiki(int cols)
 		       "|| %s || {{%s}} || {{%s}} || {{%s}} || {{%s}}"
 		       "|| %s || %s\n",
 		       (c == 1) ? "eeeeee" : "dddddd", f->vendor, f->name,
-		       f->total_size, s,
+		       f->total_size, s ? s : "?",
 		       test_state_to_template(f->tested.probe),
 		       test_state_to_template(f->tested.read),
 		       test_state_to_template(f->tested.erase),