Attention is currently required from: Simon Buhrow, Nico Huber, Paul Menzel, Aarya.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65879 )
Change subject: flashrom.c:Add function to get a flattened view of the chip erase blocks ......................................................................
Patch Set 42:
(8 comments)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/65879/comment/07d5a8df_b7c7f180 PS41, Line 1237: : struct erase_layout *create_erase_layout(struct flashctx *const flashctx); :
This function later becomes a static. […]
Would it make sense for these functions to become part of layout.c perhaps? I feel like all this specific logic is fundamentally a abstract data structure part of the project you are annotating to later consume, walk and determine the set of actions to be taken for optimality of erasure in flashrom.c.
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/65879/comment/2ce10007_6c565a1d PS42, Line 1240: s const
https://review.coreboot.org/c/flashrom/+/65879/comment/fbf292a9_70ff2c3b PS42, Line 1242: s const
https://review.coreboot.org/c/flashrom/+/65879/comment/8731a996_d5b83b1b PS42, Line 1243: layout `struct erase_layout *layout = `
and drop the cast?
Also `calloc(1, ..` avoids uninitialised heap by zeroing it.
https://review.coreboot.org/c/flashrom/+/65879/comment/64390aee_e65742e2 PS42, Line 1245: if (layout == NULL) ``` if (!ptr) return NULL; ``` is canonical.
You also want the `msg_gerr("Out of memory!\n");` message in this branch to be consistent with the rest of the codebase.
https://review.coreboot.org/c/flashrom/+/65879/comment/72662f1b_673af48e PS42, Line 1249: index doesn't `index` and `i` count in tandem? You can just use `i` and drop the `index`?
https://review.coreboot.org/c/flashrom/+/65879/comment/ebb9fd9a_5aaf89dc PS42, Line 1250: (i should be fine to scope the `i` indexer to be lexically coupled to the loop construct with `for (size_t i = 0; ..`
https://review.coreboot.org/c/flashrom/+/65879/comment/46c158ae_7577ea4b PS42, Line 1259: { `chip->block_erasers[i].eraseblocks[j]` as a intermediate to the loop pre-amble.