Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54907 )
Change subject: dummyflasher.c: Get rid of get_data_from_context() ......................................................................
dummyflasher.c: Get rid of get_data_from_context()
Relying on the global state 'dummy_buses_supported' to determine the member master struct [mst.par or mst.spi] is both buggy and ultimately unnecessary. It became apparent after commit 4eef651ff503f81b77 just how fragile this really was as the 'defaults' simultaneously selected both buses causing get_data_from_context() to fall-though however memory happened to workout by chance due to the union. With the member master structs now being struct fields the subtle bug is more apparent.
BUG=none BRANCH=none TEST=`./flashrom -r /tmp/fwupdater.apnSQQ -p dummy:emulate=VARIABLE_SIZE,image=test_update.sh.tmp.emu,size=8388608`
Change-Id: I07a34faf50ff0679cb3d6bc683142f82160010b1 Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/54907 Reviewed-by: Anastasia Klimchuk aklm@chromium.org Reviewed-by: Sam McNally sammc@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M dummyflasher.c 1 file changed, 2 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved Anastasia Klimchuk: Looks good to me, but someone else must approve
diff --git a/dummyflasher.c b/dummyflasher.c index 587fc61..b7cfab8 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -103,18 +103,6 @@ #endif #endif
-static enum chipbustype dummy_buses_supported = BUS_NONE; - -static struct emu_data* get_data_from_context(const struct flashctx *flash) -{ - if (dummy_buses_supported & BUS_NONSPI) - return (struct emu_data *)flash->mst->par.data; - else if (dummy_buses_supported & BUS_SPI) - return (struct emu_data *)flash->mst->spi.data; - - return NULL; /* buses was set to BUS_NONE. */ -} - void *dummy_map(const char *descr, uintptr_t phys_addr, size_t len) { msg_pspew("%s: Mapping %s, 0x%zx bytes at 0x%0*" PRIxPTR "\n", @@ -688,7 +676,7 @@ /* Convert the parameters to lowercase. */ tolower_string(bustext);
- dummy_buses_supported = BUS_NONE; + enum chipbustype dummy_buses_supported = BUS_NONE; if (strstr(bustext, "parallel")) { dummy_buses_supported |= BUS_PARALLEL; msg_pdbg("Enabling support for %s flash.\n", "parallel"); @@ -1041,7 +1029,7 @@ int probe_variable_size(struct flashctx *flash) { unsigned int i; - const struct emu_data *emu_data = get_data_from_context(flash); + const struct emu_data *emu_data = flash->mst->spi.data;
/* Skip the probing if we don't emulate this chip. */ if (!emu_data || emu_data->emu_chip != EMULATE_VARIABLE_SIZE)