Arthur Heymans has uploaded this change for review.

View Change

[WIP]layout.c: Use a linked lists for layout

This uses sys/queue.h to use a linked list for the struct
flashrom_layout instead of using an fix sized array.

This also removes the global variable in the layout.c file.

Change-Id: Id7f48ac613da1590f30247bee979034cf1f74e9e
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
---
M cli_classic.c
M flash.h
M flashrom.c
M ich_descriptors.c
M ich_descriptors.h
M layout.c
M layout.h
M libflashrom.c
8 files changed, 117 insertions(+), 100 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/27106/1
diff --git a/cli_classic.c b/cli_classic.c
index 119c6f8..3d5e612 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -103,7 +103,8 @@
#endif
int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0;
- struct flashrom_layout *layout = NULL;
+ struct flashrom_layout *layout = malloc(sizeof(struct flashrom_layout));
+
enum programmer prog = PROGRAMMER_INVALID;
enum {
OPTION_IFD = 0x0100,
@@ -395,11 +396,11 @@
}
msg_gdbg("\n");

- if (layoutfile && read_romlayout(layoutfile)) {
+ if (layoutfile && read_romlayout(layout, layoutfile)) {
ret = 1;
goto out;
}
- if (!ifd && process_include_args(get_global_layout())) {
+ if (!ifd && process_include_args(layout)) {
ret = 1;
goto out;
}
@@ -552,9 +553,9 @@
goto out_shutdown;
}

- if (layoutfile) {
- layout = get_global_layout();
- } else if (ifd && (flashrom_layout_read_from_ifd(&layout, fill_flash, NULL, 0) ||
+ /* if (layoutfile) { */
+ /* layout = get_global_layout(); */
+ /* } else */ if (ifd && (flashrom_layout_read_from_ifd(&layout, fill_flash, NULL, 0) ||
process_include_args(layout))) {
ret = 1;
goto out_shutdown;
@@ -590,7 +591,7 @@
for (i = 0; i < chipcount; i++)
free(flashes[i].chip);

- layout_cleanup();
+ layout_cleanup(layout);
free(filename);
free(referencefile);
free(layoutfile);
diff --git a/flash.h b/flash.h
index fca3775..2cd09c2 100644
--- a/flash.h
+++ b/flash.h
@@ -385,9 +385,9 @@

/* layout.c */
int register_include_arg(char *name);
-int read_romlayout(const char *name);
-int normalize_romentries(const struct flashctx *flash);
-void layout_cleanup(void);
+int read_romlayout(struct flashrom_layout *layout, const char *name);
+int normalize_romentries(const struct flashrom_layout *layout, const struct flashctx *flash);
+void layout_cleanup(struct flashrom_layout *layout);

/* spi.c */
struct spi_command {
diff --git a/flashrom.c b/flashrom.c
index f85dbb1..b972233 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1298,12 +1298,13 @@

/* Fill fallback layout covering the whole chip. */
struct single_layout *const fallback = &flash->fallback_layout;
- fallback->base.entries = &fallback->entry;
- fallback->base.num_entries = 1;
- fallback->entry.start = 0;
- fallback->entry.end = flash->chip->total_size * 1024 - 1;
- fallback->entry.included = true;
- strcpy(fallback->entry.name, "complete flash");
+ struct romentry entry;
+ init_layout(&fallback->base);
+ entry.start = 0;
+ entry.end = flash->chip->total_size * 1024 - 1;
+ entry.included = true;
+ strcpy(entry.name, "complete flash");
+ append_romentry(&fallback->base, &entry);

tmp = flashbuses_to_text(flash->chip->bustype);
msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found",
@@ -1559,13 +1560,14 @@
{
const struct flashrom_layout *const layout = get_layout(flashctx);

- size_t i;
- for (i = 0; i < layout->num_entries; ++i) {
- if (!layout->entries[i].included)
+ struct flashrom_layout_entry *var;
+ SLIST_FOREACH(var, layout, next) {
+ struct romentry entry = get_romentry(var);
+ if (!entry.included)
continue;

- const chipoff_t region_start = layout->entries[i].start;
- const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1;
+ const chipoff_t region_start = entry.start;
+ const chipsize_t region_len = entry.end - entry.start + 1;

if (flashctx->chip->read(flashctx, buffer + region_start, region_start, region_len))
return 1;
@@ -1646,13 +1648,14 @@
all_skipped = true;
msg_cinfo("Erasing and writing flash chip... ");

- size_t i;
- for (i = 0; i < layout->num_entries; ++i) {
- if (!layout->entries[i].included)
+ struct flashrom_layout_entry *var;
+ SLIST_FOREACH(var, layout, next) {
+ struct romentry entry = get_romentry(var);
+ if (!entry.included)
continue;

- info->region_start = layout->entries[i].start;
- info->region_end = layout->entries[i].end;
+ info->region_start = entry.start;
+ info->region_end = entry.end;

size_t j;
int error = 1; /* retry as long as it's 1 */
@@ -1861,13 +1864,14 @@
{
const struct flashrom_layout *const layout = get_layout(flashctx);

- size_t i;
- for (i = 0; i < layout->num_entries; ++i) {
- if (!layout->entries[i].included)
+ struct flashrom_layout_entry *var;
+ SLIST_FOREACH(var, layout, next) {
+ struct romentry entry = get_romentry(var);
+ if (!entry.included)
continue;

- const chipoff_t region_start = layout->entries[i].start;
- const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1;
+ const chipoff_t region_start = entry.start;
+ const chipsize_t region_len = entry.end - entry.start + 1;

if (flashctx->chip->read(flashctx, curcontents + region_start, region_start, region_len))
return 1;
@@ -2219,7 +2223,7 @@
return 1;
}

- if (flash->layout == get_global_layout() && normalize_romentries(flash)) {
+ if (normalize_romentries(flash->layout, flash)) {
msg_cerr("Requested regions can not be handled. Aborting.\n");
return 1;
}
@@ -2333,13 +2337,14 @@
{
const struct flashrom_layout *const layout = get_layout(flashctx);

- size_t i;
- for (i = 0; i < layout->num_entries; ++i) {
- if (layout->entries[i].included)
+ struct flashrom_layout_entry *var;
+ SLIST_FOREACH(var, layout, next) {
+ struct romentry entry = get_romentry(var);
+ if (!entry.included)
continue;

- const chipoff_t region_start = layout->entries[i].start;
- const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1;
+ const chipoff_t region_start = entry.start;
+ const chipsize_t region_len = entry.end - entry.start + 1;

memcpy(newcontents + region_start, oldcontents + region_start, region_len);
}
diff --git a/ich_descriptors.c b/ich_descriptors.c
index 56e846f..4c9d007 100644
--- a/ich_descriptors.c
+++ b/ich_descriptors.c
@@ -1179,8 +1179,6 @@
snprintf(layout->entries[j].name, sizeof(layout->entries[j].name), "%s", regions[i]);
++j;
}
- layout->base.entries = layout->entries;
- layout->base.num_entries = j;
return 0;
}

diff --git a/ich_descriptors.h b/ich_descriptors.h
index fa1990d..7849914 100644
--- a/ich_descriptors.h
+++ b/ich_descriptors.h
@@ -563,7 +563,7 @@
};

struct ich_layout {
- struct flashrom_layout base;
+ int num_entries;
struct romentry entries[MAX_NUM_FLREGS];
};

diff --git a/layout.c b/layout.c
index fa66238..55bed12 100644
--- a/layout.c
+++ b/layout.c
@@ -23,33 +23,41 @@
#include "programmer.h"
#include "layout.h"

-struct romentry entries[MAX_ROMLAYOUT];
-static struct flashrom_layout layout = { entries, 0 };
-
/* include_args holds the arguments specified at the command line with -i. They must be processed at some point
* so that desired regions are marked as "included" in the layout. */
static char *include_args[MAX_ROMLAYOUT];
static int num_include_args = 0; /* the number of valid include_args. */

-struct flashrom_layout *get_global_layout(void)
+void init_layout(struct flashrom_layout *layout)
{
- return &layout;
+ SLIST_INIT(layout);
+}
+
+void append_romentry(struct flashrom_layout *layout, struct romentry *entry)
+{
+ struct flashrom_layout_entry *var = malloc(sizeof(struct flashrom_layout_entry));
+ var->entry = *entry;
+ SLIST_INSERT_HEAD(layout, var, next);
+}
+
+struct romentry get_romentry(struct flashrom_layout_entry *var)
+{
+ return var->entry;
}

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 &flashctx->fallback_layout.base;
}

#ifndef __LIBPAYLOAD__
-int read_romlayout(const char *name)
+int read_romlayout(struct flashrom_layout *layout, const char *name)
{
FILE *romlayout;
char tempstr[256];
- int i;

romlayout = fopen(name, "r");

@@ -60,15 +68,10 @@
}

while (!feof(romlayout)) {
+ struct romentry entry;
char *tstr1, *tstr2;

- if (layout.num_entries >= MAX_ROMLAYOUT) {
- msg_gerr("Maximum number of ROM images (%i) in layout "
- "file reached.\n", MAX_ROMLAYOUT);
- (void)fclose(romlayout);
- return 1;
- }
- if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, layout.entries[layout.num_entries].name))
+ if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, entry.name))
continue;
#if 0
// fscanf does not like arbitrary comments like that :( later
@@ -83,16 +86,17 @@
(void)fclose(romlayout);
return 1;
}
- 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 = 0;
- layout.num_entries++;
+ entry.start = strtol(tstr1, (char **)NULL, 16);
+ entry.end = strtol(tstr2, (char **)NULL, 16);
+ entry.included = 0;
+ append_romentry(layout, &entry);
}

- for (i = 0; i < layout.num_entries; i++) {
+ struct flashrom_layout_entry *var;
+ SLIST_FOREACH(var, layout, next) {
msg_gdbg("romlayout %08x - %08x named %s\n",
- layout.entries[i].start,
- layout.entries[i].end, layout.entries[i].name);
+ var->entry.start, var->entry.end,
+ var->entry.name);
}

(void)fclose(romlayout);
@@ -135,20 +139,20 @@
return 0;
}

-/* returns the index of the entry (or a negative value if it is not found) */
+/* returns 0 if it is found or a negative value if it is not found */
static int find_romentry(struct flashrom_layout *const l, char *name)
{
- int i;
-
- if (l->num_entries == 0)
+ if (SLIST_EMPTY(l))
return -1;

msg_gspew("Looking for region \"%s\"... ", name);
- for (i = 0; i < l->num_entries; i++) {
- if (!strcmp(l->entries[i].name, name)) {
- l->entries[i].included = 1;
+
+ struct flashrom_layout_entry *var;
+ SLIST_FOREACH(var, l, next) {
+ if (!strcmp(var->entry.name, name)) {
+ var->entry.included = 1;
msg_gspew("found.\n");
- return i;
+ return 0;
}
}
msg_gspew("not found.\n");
@@ -167,7 +171,7 @@
return 0;

/* User has specified an area, but no layout file is loaded. */
- if (l->num_entries == 0) {
+ if (SLIST_EMPTY(l)) {
msg_gerr("Region requested (with -i \"%s\"), "
"but no layout data is available.\n",
include_args[0]);
@@ -191,7 +195,7 @@
return 0;
}

-void layout_cleanup(void)
+void layout_cleanup(struct flashrom_layout *layout)
{
int i;
for (i = 0; i < num_include_args; i++) {
@@ -200,29 +204,31 @@
}
num_include_args = 0;

- for (i = 0; i < layout.num_entries; i++) {
- layout.entries[i].included = 0;
+ struct flashrom_layout_entry *var;
+ while (!SLIST_EMPTY(layout)) { /* List Deletion. */
+ var = SLIST_FIRST(layout);
+ SLIST_REMOVE_HEAD(layout, next);
+ free(var);
}
- layout.num_entries = 0;
}

/* Validate and - if needed - normalize layout entries. */
-int normalize_romentries(const struct flashctx *flash)
+int normalize_romentries(const struct flashrom_layout *layout, const struct flashctx *flash)
{
chipsize_t total_size = flash->chip->total_size * 1024;
int ret = 0;

- int i;
- for (i = 0; i < layout.num_entries; i++) {
- if (layout.entries[i].start >= total_size || layout.entries[i].end >= total_size) {
+ struct flashrom_layout_entry *var;
+ SLIST_FOREACH(var, layout, next) {
+ if (var->entry.start >= total_size || var->entry.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)
+ "address space.\n", var->entry.name);
+ if (var->entry.included)
ret = 1;
}
- if (layout.entries[i].start > layout.entries[i].end) {
+ if (var->entry.start > var->entry.end) {
msg_gerr("Error: Size of the address range of region \"%s\" is not positive.\n",
- layout.entries[i].name);
+ var->entry.name);
ret = 1;
}
}
diff --git a/layout.h b/layout.h
index eb54a4f..e87d285 100644
--- a/layout.h
+++ b/layout.h
@@ -23,6 +23,7 @@

#include <stddef.h>
#include <stdint.h>
+#include <sys/queue.h>

/* Types and macros regarding the maximum flash space size supported by generic code. */
typedef uint32_t chipoff_t; /* Able to store any addressable offset within a supported flash memory. */
@@ -41,19 +42,21 @@
char name[256];
};

-struct flashrom_layout {
- /* entries store the entries specified in a layout file and associated run-time data */
- struct romentry *entries;
- /* the number of successfully parsed entries */
- size_t num_entries;
+SLIST_HEAD(flashrom_layout, flashrom_layout_entry);
+
+struct flashrom_layout_entry {
+ SLIST_ENTRY(flashrom_layout_entry) next;
+ struct romentry entry;
};

struct single_layout {
struct flashrom_layout base;
- struct romentry entry;
+// struct romentry entry;
};

-struct flashrom_layout *get_global_layout(void);
+void init_layout(struct flashrom_layout *layout);
+void append_romentry(struct flashrom_layout *layout, struct romentry *entry);
+struct romentry get_romentry(struct flashrom_layout_entry *var);
struct flashrom_flashctx;
const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx);

diff --git a/libflashrom.c b/libflashrom.c
index 34e881a..6cd919b 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -292,10 +292,10 @@
*/
int flashrom_layout_include_region(struct flashrom_layout *const layout, const char *name)
{
- size_t i;
- for (i = 0; i < layout->num_entries; ++i) {
- if (!strcmp(layout->entries[i].name, name)) {
- layout->entries[i].included = true;
+ struct flashrom_layout_entry *var;
+ SLIST_FOREACH(var, layout, next) {
+ if (!strcmp(var->entry.name, name)) {
+ var->entry.included = true;
return 0;
}
}
@@ -330,6 +330,7 @@
return 6;
#else
struct ich_layout dump_layout;
+ int i;
int ret = 1;

void *const desc = malloc(0x1000);
@@ -362,14 +363,15 @@
goto _finalize_ret;
}

- if (chip_layout->base.num_entries != dump_layout.base.num_entries ||
+ if (chip_layout->num_entries != dump_layout.num_entries ||
memcmp(chip_layout->entries, dump_layout.entries, sizeof(dump_layout.entries))) {
ret = 5;
goto _finalize_ret;
}
}

- *layout = (struct flashrom_layout *)chip_layout;
+ for (i = 0; i < chip_layout->num_entries; i++)
+ append_romentry(*layout, &chip_layout->entries[i]);
ret = 0;

_finalize_ret:
@@ -389,10 +391,12 @@
*/
void flashrom_layout_release(struct flashrom_layout *const layout)
{
- if (layout == get_global_layout())
- return;
-
- free(layout);
+ struct flashrom_layout_entry *var;
+ while (!SLIST_EMPTY(layout)) { /* List Deletion. */
+ var = SLIST_FIRST(layout);
+ SLIST_REMOVE_HEAD(layout, next);
+ free(var);
+ }
}

/**

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

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id7f48ac613da1590f30247bee979034cf1f74e9e
Gerrit-Change-Number: 27106
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz>