Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/45230 )
Change subject: dummyflasher.c: Factor out global state ......................................................................
Patch Set 1:
(6 comments)
Thanks for the patch!
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 slightly differently.
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.
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'. Also we could drop the 'emu_' prefix since the struct-typed variable will have that context already and as such we would have:
``` enum emu_chip chip; char *persistent_image; unsigned int chip_size; int modified; /* is the image modified since reading it? */ uint8_t status;
[...] ```
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@469 PS1, Line 469: } braces not needed.
https://review.coreboot.org/c/flashrom/+/45230/1/dummyflasher.c@922 PS1, Line 922: (struct emu_data *)flash->mst->par.data As we have the block:
``` if (dummy_buses_supported & (BUS_PARALLEL | BUS_LPC | BUS_FWH)) register_par_master(&par_master_dummy, dummy_buses_supported & (BUS_PARALLEL | BUS_LPC | BUS_FWH)); if (dummy_buses_supported & BUS_SPI) register_spi_master(&spi_master_dummyflasher); ```
in the init func we can't assume the mst is typed `par` in the union within mst.
A initial step would be to move this emu_data heap state extractor into its own function and then we would need to determine if we are a `par` or not and branch accordingly to return the correct ptr. You can test dummy_buses_supported just as is the case at init time.
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.