Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54748 )
Change subject: dummyflasher.c: Extract params processing into a separate function ......................................................................
Patch Set 4: Code-Review+1
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/54748/comment/82e23e4b_a1ad3e53 PS4, Line 11: A good side-effect of the change is: 13 error paths of params : processing are not leaking data anymore. All those error paths : return 1 back to init function which frees data. : : And there was just one more error path in init function were free(data) : needed to be added. These are the actual changes. Everything else is a mere refactoring. The summary line should cover this.
Extracting things into a separate function is just how the change is made easier. Maybe worth mentioning but not what should be emphasized.
https://review.coreboot.org/c/flashrom/+/54748/comment/57269786_87a3387c PS4, Line 20: Could add a
Fixes: 3b8fe0f8 (dummyflasher.c: Factor out global state)
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/54748/comment/3c1a9478_139f6599 PS1, Line 26: /* Remove the #define below if you don't want SPI flash chip emulation. */ : #define EMULATE_SPI_CHIP 1
This comment still valid, I think.
IMO, both defines should just be dropped. If they were compile-time configs like the selection of programmer drivers to built in, we could at least build test every combination. But if nobody needs different values anyway, just drop them and all the #if?
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/54748/comment/fcbfc2a0_cbe49ae4 PS4, Line 994: dummy_buses_supported Makes one wonder why this is not part of `emu_data`...