Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/27106
Change subject: [WIP]layout.c: Use a linked lists for layout ......................................................................
[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); + } }
/**
-
Arthur Heymans (Code Review)