Nico Huber submitted this change.

View Change

layout: Introduce flashrom_layout_add_region()

Adds a region to an existing layout, as long as there is space.

Change-Id: I50d473d0d5d1fb38bd6f9ae3d7127e9ea66a94e1
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/33517
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M flashrom.c
M ich_descriptors.c
M layout.c
M libflashrom.c
M libflashrom.h
5 files changed, 55 insertions(+), 50 deletions(-)

diff --git a/flashrom.c b/flashrom.c
index 8d260cb..ffba5c4 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -826,15 +826,11 @@
 	struct single_layout *const fallback = &flash->fallback_layout;
 	fallback->base.entries		= &fallback->entry;
 	fallback->base.capacity		= 1;
-	fallback->base.num_entries	= 1;
-	fallback->entry.start		= 0;
-	fallback->entry.end		= flash->chip->total_size * 1024 - 1;
-	fallback->entry.included	= true;
-	fallback->entry.name		= strdup("complete flash");
-	if (!fallback->entry.name) {
-		msg_cerr("Failed to probe chip: %s\n", strerror(errno));
+	fallback->base.num_entries	= 0;
+	if (flashrom_layout_add_region(&fallback->base,
+			0, flash->chip->total_size * 1024 - 1, "complete flash") ||
+	    flashrom_layout_include_region(&fallback->base, "complete flash"))
 		return -1;
-	}
 
 	tmp = flashbuses_to_text(flash->chip->bustype);
 	msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found",
diff --git a/ich_descriptors.c b/ich_descriptors.c
index 143e392..ad1a88b 100644
--- a/ich_descriptors.c
+++ b/ich_descriptors.c
@@ -1278,25 +1278,20 @@
 	}
 
 	memset(layout, 0x00, sizeof(*layout));
+	layout->base.entries = layout->entries;
+	layout->base.capacity = ARRAY_SIZE(layout->entries);
+	layout->base.num_entries = 0;
 
-	ssize_t i, j;
+	ssize_t i;
 	const ssize_t nr = MIN(ich_number_of_regions(cs, &desc.content), (ssize_t)ARRAY_SIZE(regions));
-	for (i = 0, j = 0; i < nr; ++i) {
+	for (i = 0; i < nr; ++i) {
 		const chipoff_t base = ICH_FREG_BASE(desc.region.FLREGs[i]);
 		const chipoff_t limit = ICH_FREG_LIMIT(desc.region.FLREGs[i]);
 		if (limit <= base)
 			continue;
-		layout->entries[j].start = base;
-		layout->entries[j].end = limit;
-		layout->entries[j].included = false;
-		layout->entries[j].name = strdup(regions[i]);
-		if (!layout->entries[j].name)
+		if (flashrom_layout_add_region(&layout->base, base, limit, regions[i]))
 			return 2;
-		++j;
 	}
-	layout->base.entries = layout->entries;
-	layout->base.capacity = ARRAY_SIZE(layout->entries);
-	layout->base.num_entries = j;
 	return 0;
 }
 
diff --git a/layout.c b/layout.c
index 0771d42..2f56b27 100644
--- a/layout.c
+++ b/layout.c
@@ -73,7 +73,6 @@
 	struct flashrom_layout *const layout = get_global_layout();
 	FILE *romlayout;
 	char tempstr[256], tempname[256];
-	unsigned int i;
 	int ret = 1;
 
 	romlayout = fopen(name, "r");
@@ -106,24 +105,10 @@
 			msg_gerr("Error parsing layout file. Offending string: \"%s\"\n", tempstr);
 			goto _close_ret;
 		}
-		layout->entries[layout->num_entries].start = strtol(tstr1, (char **)NULL, 16);
-		layout->entries[layout->num_entries].end = strtol(tstr2, (char **)NULL, 16);
-		layout->entries[layout->num_entries].included = false;
-		layout->entries[layout->num_entries].file = NULL;
-		layout->entries[layout->num_entries].name = strdup(tempname);
-		if (!layout->entries[layout->num_entries].name) {
-			msg_gerr("Error adding layout entry: %s\n", strerror(errno));
+		if (flashrom_layout_add_region(layout,
+				strtol(tstr1, NULL, 16), strtol(tstr2, NULL, 16), tempname))
 			goto _close_ret;
-		}
-		layout->num_entries++;
 	}
-
-	for (i = 0; i < layout->num_entries; i++) {
-		msg_gdbg("romlayout %08x - %08x named %s\n",
-			     layout->entries[i].start,
-			     layout->entries[i].end, layout->entries[i].name);
-	}
-
 	ret = 0;
 
 _close_ret:
@@ -399,6 +384,43 @@
  */
 
 /**
+ * @brief Add another region to an existing layout.
+ *
+ * @param layout The existing layout.
+ * @param start  Start address of the region.
+ * @param end    End address (inclusive) of the region.
+ * @param name   Name of the region.
+ *
+ * @return 0 on success,
+ *         1 if out of memory,
+ *         2 if the layout is full already.
+ */
+int flashrom_layout_add_region(
+		struct flashrom_layout *const layout,
+		const size_t start, const size_t end, const char *const name)
+{
+	if (layout->num_entries >= layout->capacity) {
+		msg_gerr("Error adding layout entry: No space left\n");
+		return 2;
+	}
+
+	struct romentry *const entry = &layout->entries[layout->num_entries];
+	entry->start	= start;
+	entry->end	= end;
+	entry->included	= false;
+	entry->name	= strdup(name);
+	entry->file	= NULL;
+	if (!entry->name) {
+		msg_gerr("Error adding layout entry: %s\n", strerror(errno));
+		return 1;
+	}
+
+	msg_gdbg("Added layout entry %08zx - %08zx named %s\n", start, end, name);
+	++layout->num_entries;
+	return 0;
+}
+
+/**
  * @brief Mark given region as included.
  *
  * @param layout The layout to alter.
diff --git a/libflashrom.c b/libflashrom.c
index 2a39a80..c5c4c89 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -501,6 +501,8 @@
 		struct flashctx *const flashctx, const struct fmap *const fmap)
 {
 	int i;
+	char name[FMAP_STRLEN + 1];
+	const struct fmap_area *area;
 	struct flashrom_layout *l = get_global_layout();
 
 	if (!fmap || !l)
@@ -511,21 +513,10 @@
 		return 1;
 	}
 
-	for (i = 0; i < fmap->nareas; i++) {
-		l->entries[l->num_entries].start = fmap->areas[i].offset;
-		l->entries[l->num_entries].end = fmap->areas[i].offset + fmap->areas[i].size - 1;
-		l->entries[l->num_entries].included = false;
-		l->entries[l->num_entries].name =
-			strndup((const char *)fmap->areas[i].name, FMAP_STRLEN);
-		if (!l->entries[l->num_entries].name) {
-			msg_gerr("Error adding layout entry: %s\n", strerror(errno));
+	for (i = 0, area = fmap->areas; i < fmap->nareas; i++, area++) {
+		snprintf(name, sizeof(name), "%s", area->name);
+		if (flashrom_layout_add_region(l, area->offset, area->offset + area->size - 1, name))
 			return 1;
-		}
-		msg_gdbg("fmap %08x - %08x named %s\n",
-			l->entries[l->num_entries].start,
-			l->entries[l->num_entries].end,
-			l->entries[l->num_entries].name);
-		l->num_entries++;
 	}
 
 	*layout = l;
diff --git a/libflashrom.h b/libflashrom.h
index eab78ad..d04eeb6 100644
--- a/libflashrom.h
+++ b/libflashrom.h
@@ -111,6 +111,7 @@
 		struct flashrom_flashctx *, off_t offset, size_t length);
 int flashrom_layout_read_fmap_from_buffer(struct flashrom_layout **layout,
 		struct flashrom_flashctx *, const uint8_t *buf, size_t len);
+int flashrom_layout_add_region(struct flashrom_layout *, size_t start, size_t end, const char *name);
 int flashrom_layout_include_region(struct flashrom_layout *, const char *name);
 void flashrom_layout_release(struct flashrom_layout *);
 void flashrom_layout_set(struct flashrom_flashctx *, const struct flashrom_layout *);

To view, visit change 33517. To unsubscribe, or for help writing mail filters, visit settings.