Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69196 )
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.
Change-Id: I768742b73db901df5b5208fcbcb8a324a06014c2 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M cli_classic.c M flashrom.c M include/layout.h M layout.c 4 files changed, 68 insertions(+), 34 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/69196/1
diff --git a/cli_classic.c b/cli_classic.c index b66094c..72ab35b 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -385,8 +385,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; @@ -411,8 +412,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 d972e29..ca2c46c 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1007,8 +1007,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; @@ -1169,8 +1170,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 */ @@ -1445,8 +1447,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; @@ -1785,12 +1788,12 @@ chipoff_t start = 0;
while ((included = layout_next_included_region(layout, start))) { - if (included->start > start) { + if (included->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, included->region.start - start); } /* skip this included region */ - start = included->end + 1; + start = included->region.end + 1; if (start == 0) return; } diff --git a/include/layout.h b/include/layout.h index abbdc22..7ff75bf 100644 --- a/include/layout.h +++ b/include/layout.h @@ -35,14 +35,21 @@
#define MAX_ROMLAYOUT 128
+struct flash_region { + char *name; + unsigned int start; + unsigned int end; + bool read_prot; + bool write_prot; +}; + 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 be88428..0fb1763 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; @@ -250,14 +250,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; } } @@ -285,15 +288,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; } } @@ -311,7 +315,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])) @@ -328,9 +332,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; }
@@ -374,14 +378,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); @@ -404,8 +410,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; @@ -420,7 +427,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);
-
Edward O'Callaghan (Code Review)