Nico Huber submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved
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 <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/33521
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
---
M layout.c
M layout.h
M libflashrom.c
3 files changed, 51 insertions(+), 71 deletions(-)

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

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

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I60a0aa1007ebcd5eb401db116f835d129b3e9732
Gerrit-Change-Number: 33521
Gerrit-PatchSet: 11
Gerrit-Owner: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Daniel Campello <campello@chromium.org>
Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Peter Marheine <pmarheine@chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src@posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@mailbox.org>
Gerrit-MessageType: merged