Change in flashrom[master]: layout: Use linked list for `struct romentry`

[Nico Huber (Code Review)]

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

Change subject: layout: Use linked list for `struct romentry`
......................................................................

layout: Use linked list for `struct romentry`

This gets rid of the entry limit and hopefully makes future layout
handling easier. We start by making `struct flashrom_layout` private
to `layout.c`.

Change-Id: I60a0aa1007ebcd5eb401db116f835d129b3e9732
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/33521
Tested-by: build bot (Jenkins) 
Reviewed-by: Edward O'Callaghan 
Reviewed-by: Angel Pons 
---
M layout.c
M layout.h
M libflashrom.c
3 files changed, 51 insertions(+), 71 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved
  Edward O'Callaghan: Looks good to me, approved

diff --git a/layout.c b/layout.c
index 11df15c..8047a93 100644
--- a/layout.c
+++ b/layout.c
@@ -25,12 +25,17 @@
 #include "programmer.h"
 #include "layout.h"

-static struct romentry entries[MAX_ROMLAYOUT];
-static struct flashrom_layout global_layout = { entries, MAX_ROMLAYOUT, 0 };
+struct flashrom_layout {
+	struct romentry *head;
+};
+
+static struct flashrom_layout *global_layout;

struct flashrom_layout *get_global_layout(void)
 {
-	return &global_layout;
+	if (!global_layout)
+		flashrom_layout_new(&global_layout, 0);
+	return global_layout;
 }

const struct flashrom_layout *get_default_layout(const struct flashrom_flashctx *const flashctx)
@@ -40,7 +45,7 @@

const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx)
 {
-	if (flashctx->layout && flashctx->layout->num_entries)
+	if (flashctx->layout)
    	return flashctx->layout;
    else
    	return get_default_layout(flashctx);
@@ -49,16 +54,7 @@
 static struct romentry *mutable_layout_next(
    	const struct flashrom_layout *const layout, struct romentry *iterator)
 {
-	const struct romentry *const end = layout->entries + layout->num_entries;
-
-	if (iterator)
-		++iterator;
-	else
-		iterator = &layout->entries[0];
-
-	if (iterator < end)
-		return iterator;
-	return NULL;
+	return iterator ? iterator->next : layout->head;
 }

static struct romentry *_layout_entry_by_name(
@@ -91,11 +87,6 @@
    while (!feof(romlayout)) {
    	char *tstr1, *tstr2;

-		if (layout->num_entries >= layout->capacity) {
-			msg_gerr("Maximum number of ROM images (%zu) in layout "
-				 "file reached.\n", layout->capacity);
-			goto _close_ret;
-		}
    	if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, tempname))
    		continue;
 #if 0
@@ -186,7 +177,7 @@
 /* returns -1 if an entry is not found, 0 if found. */
 static int find_romentry(struct flashrom_layout *const l, char *name, char *file)
 {
-	if (l->num_entries == 0)
+	if (!l->head)
    	return -1;

msg_gspew("Looking for region "%s"... ", name);
@@ -222,7 +213,7 @@
    	return 0;

/* User has specified an include argument, but no layout is loaded. */
-	if (l->num_entries == 0) {
+	if (!l->head) {
    	msg_gerr("Region requested (with -i "%s"), "
    		 "but no layout data is available.\n",
    		 args->name);
@@ -287,7 +278,6 @@
 void layout_cleanup(struct layout_include_args **args)
 {
    struct flashrom_layout *const layout = get_global_layout();
-	unsigned int i;
    struct layout_include_args *tmp;

while (*args) {
@@ -298,12 +288,8 @@
    	*args = tmp;
    }

-	for (i = 0; i < layout->num_entries; i++) {
-		free(layout->entries[i].name);
-		free(layout->entries[i].file);
-		layout->entries[i].included = false;
-	}
-	layout->num_entries = 0;
+	global_layout = NULL;
+	flashrom_layout_release(layout);
 }

/* Validate and - if needed - normalize layout entries. */
@@ -380,7 +366,7 @@
 const struct romentry *layout_next(
    	const struct flashrom_layout *const layout, const struct romentry *iterator)
 {
-	return mutable_layout_next(layout, (struct romentry *)iterator);
+	return iterator ? iterator->next : layout->head;
 }

/**
@@ -399,17 +385,13 @@
  */
 int flashrom_layout_new(struct flashrom_layout **const layout, const unsigned int count)
 {
-	*layout = malloc(sizeof(**layout) + count * sizeof(struct romentry));
+	*layout = malloc(sizeof(**layout));
    if (!*layout) {
    	msg_gerr("Error creating layout: %s\n", strerror(errno));
    	return 1;
    }

-	const struct flashrom_layout tmp = {
-		.entries	= (void *)((char *)*layout + sizeof(**layout)),
-		.capacity	= count,
-		.num_entries	= 0,
-	};
+	const struct flashrom_layout tmp = { 0 };
    **layout = tmp;
    return 0;
 }
@@ -423,32 +405,36 @@
  * @param name   Name of the region.
  *
  * @return 0 on success,
- *         1 if out of memory,
- *         2 if the layout is full already.
+ *         1 if out of memory.
  */
 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 = malloc(sizeof(*entry));
+	if (!entry)
+		goto _err_ret;

-	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;
-	}
+	const struct romentry tmp = {
+		.next		= layout->head,
+		.start		= start,
+		.end		= end,
+		.included	= false,
+		.name		= strdup(name),
+		.file		= NULL,
+	};
+	*entry = tmp;
+	if (!entry->name)
+		goto _err_ret;

msg_gdbg("Added layout entry %08zx - %08zx named %s\n", start, end, name);
-	++layout->num_entries;
+	layout->head = entry;
    return 0;
+
+_err_ret:
+	msg_gerr("Error adding layout entry: %s\n", strerror(errno));
+	free(entry);
+	return 1;
 }

/**
@@ -472,14 +458,18 @@
  */
 void flashrom_layout_release(struct flashrom_layout *const layout)
 {
-	unsigned int i;
-
-	if (!layout || layout == get_global_layout())
+	if (layout == global_layout)
    	return;

-	for (i = 0; i < layout->num_entries; ++i) {
-		free(layout->entries[i].name);
-		free(layout->entries[i].file);
+	if (!layout)
+		return;
+
+	while (layout->head) {
+		struct romentry *const entry = layout->head;
+		layout->head = entry->next;
+		free(entry->file);
+		free(entry->name);
+		free(entry);
    }
    free(layout);
 }
diff --git a/layout.h b/layout.h
index 49c1a13..bf183a6 100644
--- a/layout.h
+++ b/layout.h
@@ -36,6 +36,8 @@
 #define MAX_ROMLAYOUT	128

struct romentry {
+	struct romentry *next;
+
    chipoff_t start;
    chipoff_t end;
    bool included;
@@ -43,14 +45,7 @@
    char *file;
 };

-struct flashrom_layout {
-	/* entries store the entries specified in a layout file and associated run-time data */
-	struct romentry *entries;
-	/* the maximum number of entries */
-	size_t capacity;
-	/* the number of successfully parsed entries */
-	size_t num_entries;
-};
+struct flashrom_layout;

struct layout_include_args {
    char *name;
diff --git a/libflashrom.c b/libflashrom.c
index dd5cb00..cb0d470 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -505,11 +505,6 @@
    if (!fmap || !l)
    	return 1;

-	if (l->num_entries + fmap->nareas > l->capacity) {
-		msg_gerr("Cannot add fmap entries to layout - Too many entries.\n");
-		return 1;
-	}
-
    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))