From 66655b74236eb01ac0fd21ee11b5082ea720ed6b Mon Sep 17 00:00:00 2001 From: Nikolai Artemiev Date: Wed, 7 Dec 2022 11:16:06 +1100 Subject: layout: Factor out flash_region structure from romentry The romentry structure is the container ADT with some annotated meta-data such as 'included' or 'file' however the substantive substructure is a 'flash_region'. Therefore factor this out. That is to say, the link list node 'romentry' is obscured by the implementation details of its use-case of 'flash_region' that we clear up here. BUG=b:260440773 BRANCH=none TEST=flashrom_tester Change-Id: I768742b73db901df5b5208fcbcb8a324a06014c2 CoAuthored-by: Nikolai Artemiev Signed-off-by: Nikolai Artemiev Signed-off-by: Edward O'Callaghan Reviewed-on: https://review.coreboot.org/c/flashrom/+/69196 Reviewed-by: Edward O'Callaghan Reviewed-by: Anastasia Klimchuk Tested-by: build bot (Jenkins) --- cli_classic.c | 10 ++++++---- flashrom.c | 22 +++++++++++++--------- include/layout.h | 11 ++++++++--- layout.c | 43 +++++++++++++++++++++++++------------------ 4 files changed, 52 insertions(+), 34 deletions(-) diff --git a/cli_classic.c b/cli_classic.c index 0b0944f30..c72836fc7 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -388,8 +388,9 @@ static int read_buf_from_include_args(const struct flashrom_layout *const layout while ((entry = layout_next_included(layout, entry))) { if (!entry->file) continue; - if (read_buf_from_file(buf + entry->start, - entry->end - entry->start + 1, entry->file)) + const struct flash_region *region = &entry->region; + if (read_buf_from_file(buf + region->start, + region->end - region->start + 1, entry->file)) return 1; } return 0; @@ -414,8 +415,9 @@ static int write_buf_to_include_args(const struct flashrom_layout *const layout, while ((entry = layout_next_included(layout, entry))) { if (!entry->file) continue; - if (write_buf_to_file(buf + entry->start, - entry->end - entry->start + 1, entry->file)) + const struct flash_region *region = &entry->region; + if (write_buf_to_file(buf + region->start, + region->end - region->start + 1, entry->file)) return 1; } diff --git a/flashrom.c b/flashrom.c index 28201a703..e271614a9 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1029,8 +1029,9 @@ static int read_by_layout(struct flashctx *const flashctx, uint8_t *const buffer const struct romentry *entry = NULL; while ((entry = layout_next_included(layout, entry))) { - const chipoff_t region_start = entry->start; - const chipsize_t region_len = entry->end - entry->start + 1; + const struct flash_region *region = &entry->region; + const chipoff_t region_start = region->start; + const chipsize_t region_len = region->end - region->start + 1; if (read_flash(flashctx, buffer + region_start, region_start, region_len)) return 1; @@ -1191,8 +1192,9 @@ static int walk_by_layout(struct flashctx *const flashctx, struct walk_info *con msg_cinfo("Erasing and writing flash chip... "); while ((entry = layout_next_included(layout, entry))) { - info->region_start = entry->start; - info->region_end = entry->end; + const struct flash_region *region = &entry->region; + info->region_start = region->start; + info->region_end = region->end; size_t j; int error = 1; /* retry as long as it's 1 */ @@ -1467,8 +1469,9 @@ static int verify_by_layout( const struct romentry *entry = NULL; while ((entry = layout_next_included(layout, entry))) { - const chipoff_t region_start = entry->start; - const chipsize_t region_len = entry->end - entry->start + 1; + const struct flash_region *region = &entry->region; + const chipoff_t region_start = region->start; + const chipsize_t region_len = region->end - region->start + 1; if (read_flash(flashctx, curcontents + region_start, region_start, region_len)) return 1; @@ -1807,12 +1810,13 @@ static void combine_image_by_layout(const struct flashctx *const flashctx, chipoff_t start = 0; while ((included = layout_next_included_region(layout, start))) { - if (included->start > start) { + const struct flash_region *region = &included->region; + if (region->start > start) { /* copy everything up to the start of this included region */ - memcpy(newcontents + start, oldcontents + start, included->start - start); + memcpy(newcontents + start, oldcontents + start, region->start - start); } /* skip this included region */ - start = included->end + 1; + start = region->end + 1; if (start == 0) return; } diff --git a/include/layout.h b/include/layout.h index abbdc22c1..6959ef743 100644 --- a/include/layout.h +++ b/include/layout.h @@ -35,14 +35,19 @@ typedef uint32_t chipsize_t; /* Able to store the number of bytes of any support #define MAX_ROMLAYOUT 128 +struct flash_region { + char *name; + chipoff_t start; + chipoff_t end; +}; + struct romentry { struct romentry *next; - chipoff_t start; - chipoff_t end; bool included; - char *name; char *file; + + struct flash_region region; }; struct flashrom_layout; diff --git a/layout.c b/layout.c index 3f2abb9a3..6af0e41d2 100644 --- a/layout.c +++ b/layout.c @@ -61,7 +61,7 @@ static struct romentry *_layout_entry_by_name( if (!layout || !name) return NULL; while ((entry = mutable_layout_next(layout, entry))) { - if (!strcmp(entry->name, name)) + if (!strcmp(entry->region.name, name)) return entry; } return NULL; @@ -283,14 +283,17 @@ int included_regions_overlap(const struct flashrom_layout *const l) if (!rhs->included) continue; - if (lhs->start > rhs->end) + const struct flash_region *rhsr = &rhs->region; + const struct flash_region *lhsr = &lhs->region; + + if (lhsr->start > rhsr->end) continue; - if (lhs->end < rhs->start) + if (lhsr->end < rhsr->start) continue; msg_gwarn("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); + lhsr->name, lhsr->start, lhsr->end, rhsr->name, rhsr->start, rhsr->end); overlap_detected = 1; } } @@ -318,15 +321,16 @@ int layout_sanity_checks(const struct flashrom_flashctx *const flash) const struct romentry *entry = NULL; while ((entry = layout_next(layout, entry))) { - if (entry->start >= total_size || entry->end >= total_size) { + const struct flash_region *region = &entry->region; + if (region->start >= total_size || region->end >= total_size) { msg_gwarn("Warning: Address range of region \"%s\" " - "exceeds the current chip's address space.\n", entry->name); + "exceeds the current chip's address space.\n", region->name); if (entry->included) ret = 1; } - if (entry->start > entry->end) { + if (region->start > region->end) { msg_gerr("Error: Size of the address range of region \"%s\" is not positive.\n", - entry->name); + region->name); ret = 1; } } @@ -344,7 +348,7 @@ void prepare_layout_for_extraction(struct flashctx *flash) entry->included = true; if (!entry->file) - entry->file = strdup(entry->name); + entry->file = strdup(entry->region.name); for (i = 0; entry->file[i]; ++i) { if (isspace((unsigned char)entry->file[i])) @@ -361,9 +365,9 @@ const struct romentry *layout_next_included_region( while ((entry = layout_next(l, entry))) { if (!entry->included) continue; - if (entry->end < where) + if (entry->region.end < where) continue; - if (!lowest || lowest->start > entry->start) + if (!lowest || lowest->region.start > entry->region.start) lowest = entry; } @@ -407,14 +411,16 @@ int flashrom_layout_add_region( const struct romentry tmp = { .next = layout->head, - .start = start, - .end = end, .included = false, - .name = strdup(name), .file = NULL, + .region = { + .start = start, + .end = end, + .name = strdup(name), + }, }; *entry = tmp; - if (!entry->name) + if (!entry->region.name) goto _err_ret; msg_gdbg("Added layout entry %08zx - %08zx named %s\n", start, end, name); @@ -437,8 +443,9 @@ int flashrom_layout_get_region_range(struct flashrom_layout *const l, const char { const struct romentry *const entry = _layout_entry_by_name(l, name); if (entry) { - *start = entry->start; - *len = entry->end - entry->start + 1; + const struct flash_region *region = &entry->region; + *start = region->start; + *len = region->end - region->start + 1; return 0; } return 1; @@ -453,7 +460,7 @@ void flashrom_layout_release(struct flashrom_layout *const layout) struct romentry *const entry = layout->head; layout->head = entry->next; free(entry->file); - free(entry->name); + free(entry->region.name); free(entry); } free(layout); -- cgit v1.2.3