Change in flashrom[master]: layout: Introduce flashrom_layout_add_region()

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/33517 )

Change subject: layout: Introduce flashrom_layout_add_region()
......................................................................

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 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/33517
Reviewed-by: Angel Pons 
Reviewed-by: Anastasia Klimchuk 
Reviewed-by: Peter Marheine 
Reviewed-by: Edward O'Callaghan 
Tested-by: build bot (Jenkins) 
---
M flashrom.c
M ich_descriptors.c
M layout.c
M libflashrom.c
M libflashrom.h
5 files changed, 55 insertions(+), 50 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved
  Edward O'Callaghan: Looks good to me, approved
  Peter Marheine: Looks good to me, but someone else must approve
  Anastasia Klimchuk: Looks good to me, approved

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 *);