Lachlan Bishop has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/45230 )
Change subject: dummyflasher.c: Factor out global state ......................................................................
Patch Set 3:
(9 comments)
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@40 PS1, Line 40: tatic uint8_t *flashchip_contents = NULL;
more global state here although you could deal with that as a follow up as well since its used sligh […]
Ack
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@56 PS1, Line 56: #if EMULATE_SPI_CHIP
We probably don't need to do this inside the type definition.
Done
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@68 PS1, Line 68: uint8_t emu_status;
This could be a follow up, but to move this below 'emu_modified'. […]
Ack
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@175 PS1, Line 175: data->emu_chip = EMULATE_NONE;
This will trigger undefined behavior if `data` is null. […]
Done
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@467 PS1, Line 467: {
These braces weren't here before
Done
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@469 PS1, Line 469: }
braces not needed.
Done
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@542 PS1, Line 542: emu_data
This was named `data` in `dummy_init`, why use a different name for the same thing here?
Done
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@922 PS1, Line 922: (struct emu_data *)flash->mst->par.data
For clarity, […]
Done
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@967 PS1, Line 967: const struct emu_data *emu_data = (struct emu_data *)flash->mst->par.data;
ditto.
Done