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 <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(-) 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)) -- To view, visit https://review.coreboot.org/c/flashrom/+/33521 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/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