Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69196 )
(
8 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: layout: Factor out flash_region structure from romentry ......................................................................
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 nartemiev@google.com Signed-off-by: Nikolai Artemiev nartemiev@google.com Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/69196 Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: Anastasia Klimchuk aklm@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M cli_classic.c M flashrom.c M include/layout.h M layout.c 4 files changed, 80 insertions(+), 34 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved
diff --git a/cli_classic.c b/cli_classic.c index 0b0944f..c72836f 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -388,8 +388,9 @@ 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 @@ 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 28201a7..e271614 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1029,8 +1029,9 @@ 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 @@ 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 @@ 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 @@ 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 abbdc22..6959ef7 100644 --- a/include/layout.h +++ b/include/layout.h @@ -35,14 +35,19 @@
#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 3f2abb9..6af0e41 100644 --- a/layout.c +++ b/layout.c @@ -61,7 +61,7 @@ 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 @@ 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 @@
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 @@ 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 @@ 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 @@
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 @@ { 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 @@ struct romentry *const entry = layout->head; layout->head = entry->next; free(entry->file); - free(entry->name); + free(entry->region.name); free(entry); } free(layout);