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 Peter Marheine: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved
layout: Introduce layout_next()

Also, a `layout.c` internal version mutable_layout_next() that
allows to modify layout entries and a shorthand to look up an
entry by name, _layout_entry_by_name().

Use the new functions where applicable and the code is not
dropped later in this train, and also to compare the layouts
in flashrom_layout_read_from_ifd() in depth.

Change-Id: I284958471c61344d29d92c95d88475065a9ca9aa
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/33542
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 layout.c
M layout.h
M libflashrom.c
3 files changed, 88 insertions(+), 66 deletions(-)

diff --git a/layout.c b/layout.c
index cd6b458..2446e8f 100644
--- a/layout.c
+++ b/layout.c
@@ -41,6 +41,32 @@
return &flashctx->fallback_layout.base;
}

+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;
+}
+
+static struct romentry *_layout_entry_by_name(
+ const struct flashrom_layout *const layout, const char *name)
+{
+ struct romentry *entry = NULL;
+ while ((entry = mutable_layout_next(layout, entry))) {
+ if (!strcmp(entry->name, name))
+ return entry;
+ }
+ return NULL;
+}
+
#ifndef __LIBPAYLOAD__
int read_romlayout(const char *name)
{
@@ -157,14 +183,12 @@
static int include_region(struct flashrom_layout *const l, const char *name,
const char *file)
{
- size_t i;
- for (i = 0; i < l->num_entries; ++i) {
- if (!strcmp(l->entries[i].name, name)) {
- l->entries[i].included = true;
- if (file)
- l->entries[i].file = strdup(file);
- return 0;
- }
+ struct romentry *const entry = _layout_entry_by_name(l, name);
+ if (entry) {
+ entry->included = true;
+ if (file)
+ entry->file = strdup(file);
+ return 0;
}
return 1;
}
@@ -187,13 +211,11 @@
int get_region_range(struct flashrom_layout *const l, const char *name,
unsigned int *start, unsigned int *len)
{
- size_t i;
- for (i = 0; i < l->num_entries; ++i) {
- if (!strcmp(l->entries[i].name, name)) {
- *start = l->entries[i].start;
- *len = l->entries[i].end - l->entries[i].start + 1;
- return 0;
- }
+ const struct romentry *const entry = _layout_entry_by_name(l, name);
+ if (entry) {
+ *start = entry->start;
+ *len = entry->end - entry->start + 1;
+ return 0;
}
return 1;
}
@@ -246,30 +268,26 @@
/* returns boolean 1 if any regions overlap, 0 otherwise */
int included_regions_overlap(const struct flashrom_layout *const l)
{
- size_t i;
+ const struct romentry *lhs = NULL;
int overlap_detected = 0;

- for (i = 0; i < l->num_entries; i++) {
- size_t j;
-
- if (!l->entries[i].included)
+ while ((lhs = layout_next(l, lhs))) {
+ if (!lhs->included)
continue;

- for (j = i + 1; j < l->num_entries; j++) {
- if (!l->entries[j].included)
+ const struct romentry *rhs = lhs;
+ while ((rhs = layout_next(l, rhs))) {
+ if (rhs->included)
continue;

- if (l->entries[i].start > l->entries[j].end)
+ if (lhs->start > rhs->end)
continue;

- if (l->entries[i].end < l->entries[j].start)
+ if (lhs->end < rhs->start)
continue;

- msg_gdbg("Regions %s [0x%08x-0x%08x] and "
- "%s [0x%08x-0x%08x] overlap\n",
- l->entries[i].name, l->entries[i].start,
- l->entries[i].end, l->entries[j].name,
- l->entries[j].start, l->entries[j].end);
+ msg_gdbg("Regions %s [0x%08x-0x%08x] and %s [0x%08x-0x%08x] overlap\n",
+ lhs->name, lhs->start, lhs->end, rhs->name, rhs->start, rhs->end);
overlap_detected = 1;
}
}
@@ -305,17 +323,17 @@
chipsize_t total_size = flash->chip->total_size * 1024;
int ret = 0;

- unsigned int i;
- for (i = 0; i < layout->num_entries; i++) {
- if (layout->entries[i].start >= total_size || layout->entries[i].end >= total_size) {
- msg_gwarn("Warning: Address range of region \"%s\" exceeds the current chip's "
- "address space.\n", layout->entries[i].name);
- if (layout->entries[i].included)
+ const struct romentry *entry = NULL;
+ while ((entry = layout_next(layout, entry))) {
+ if (entry->start >= total_size || entry->end >= total_size) {
+ msg_gwarn("Warning: Address range of region \"%s\" "
+ "exceeds the current chip's address space.\n", entry->name);
+ if (entry->included)
ret = 1;
}
- if (layout->entries[i].start > layout->entries[i].end) {
+ if (entry->start > entry->end) {
msg_gerr("Error: Size of the address range of region \"%s\" is not positive.\n",
- layout->entries[i].name);
+ entry->name);
ret = 1;
}
}
@@ -326,17 +344,18 @@
void prepare_layout_for_extraction(struct flashctx *flash)
{
const struct flashrom_layout *const l = get_layout(flash);
- unsigned int i, j;
+ struct romentry *entry = NULL;
+ unsigned int i;

- for (i = 0; i < l->num_entries; ++i) {
- l->entries[i].included = true;
+ while ((entry = mutable_layout_next(l, entry))) {
+ entry->included = true;

- if (!l->entries[i].file)
- l->entries[i].file = strdup(l->entries[i].name);
+ if (!entry->file)
+ entry->file = strdup(entry->name);

- for (j = 0; l->entries[i].file[j]; j++) {
- if (isspace(l->entries[i].file[j]))
- l->entries[i].file[j] = '_';
+ for (i = 0; entry->file[i]; ++i) {
+ if (isspace(entry->file[i]))
+ entry->file[i] = '_';
}
}
}
@@ -344,16 +363,15 @@
const struct romentry *layout_next_included_region(
const struct flashrom_layout *const l, const chipoff_t where)
{
- unsigned int i;
- const struct romentry *lowest = NULL;
+ const struct romentry *entry = NULL, *lowest = NULL;

- for (i = 0; i < l->num_entries; ++i) {
- if (!l->entries[i].included)
+ while ((entry = layout_next(l, entry))) {
+ if (!entry->included)
continue;
- if (l->entries[i].end < where)
+ if (entry->end < where)
continue;
- if (!lowest || lowest->start > l->entries[i].start)
- lowest = &l->entries[i];
+ if (!lowest || lowest->start > entry->start)
+ lowest = entry;
}

return lowest;
@@ -362,19 +380,17 @@
const struct romentry *layout_next_included(
const struct flashrom_layout *const layout, const struct romentry *iterator)
{
- const struct romentry *const end = layout->entries + layout->num_entries;
-
- if (iterator)
- ++iterator;
- else
- iterator = &layout->entries[0];
-
- for (; iterator < end; ++iterator) {
- if (!iterator->included)
- continue;
- return iterator;
+ while ((iterator = layout_next(layout, iterator))) {
+ if (iterator->included)
+ break;
}
- return NULL;
+ return iterator;
+}
+
+const struct romentry *layout_next(
+ const struct flashrom_layout *const layout, const struct romentry *iterator)
+{
+ return mutable_layout_next(layout, (struct romentry *)iterator);
}

/**
diff --git a/layout.h b/layout.h
index f574097..d60c89e 100644
--- a/layout.h
+++ b/layout.h
@@ -70,6 +70,7 @@
int process_include_args(struct flashrom_layout *l, const struct layout_include_args *const args);
const struct romentry *layout_next_included_region(const struct flashrom_layout *, chipoff_t);
const struct romentry *layout_next_included(const struct flashrom_layout *, const struct romentry *);
+const struct romentry *layout_next(const struct flashrom_layout *, const struct romentry *);
int included_regions_overlap(const struct flashrom_layout *const flashrom_layout);
void prepare_layout_for_extraction(struct flashrom_flashctx *flash);

diff --git a/libflashrom.c b/libflashrom.c
index d999efc..c6a598b 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -470,8 +470,13 @@
goto _finalize_ret;
}

- if (chip_layout->base.num_entries != dump_layout.base.num_entries ||
- memcmp(chip_layout->entries, dump_layout.entries, sizeof(dump_layout.entries))) {
+ const struct romentry *chip_entry = layout_next(&chip_layout->base, NULL);
+ const struct romentry *dump_entry = layout_next(&dump_layout.base, NULL);
+ while (chip_entry && dump_entry && !memcmp(chip_entry, dump_entry, sizeof(*chip_entry))) {
+ chip_entry = layout_next(&chip_layout->base, chip_entry);
+ dump_entry = layout_next(&dump_layout.base, dump_entry);
+ }
+ if (chip_entry || dump_entry) {
msg_cerr("Descriptors don't match!\n");
ret = 5;
goto _finalize_ret;

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

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I284958471c61344d29d92c95d88475065a9ca9aa
Gerrit-Change-Number: 33542
Gerrit-PatchSet: 9
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