Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57437 )
Change subject: tests: Revise mock chip definition and usage ......................................................................
Patch Set 4:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/57437/comment/686afcec_dd69fa36 PS4, Line 24: fresh copy on the stack. This made me chuckle a little :) What I always tell people when the commit message is a longer list: This list perfectly tells us what separate commits this could have been. More, smaller commits are usually cheaper.
But please don't split this one, as it's almost through review ;)
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/57437/comment/00c4c9b2_4c4c4ab2 PS3, Line 24: #define CHIP_TOTAL_SIZE 8388608
Wow useful macros. I didn't know of them, probably because I was looking into flashchips. […]
Makes sense.
https://review.coreboot.org/c/flashrom/+/57437/comment/9e296448_9d9a9219 PS3, Line 128: static struct flashchip chip_8MiB = { Looks good.
For example, it is not a deep copy.
Hmmm, let's have a look at that. We also use a shallow copy of the entries in the `flashchips.c` list (flashrom.c:787).
Looking for pointers among the struct members, they are all pointing to `const` strings or functions. The strings don't have to, but in this case are global i.e. not dynamic memory, same for functions. There's one exception, the `wp` member. But I'm not sure if it is non-const on purpose. It's part of some WIP development, so I would ignore it for now.