Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66684 )
Change subject: flashrom.c: create is_internal_programmer() helper ......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
I am wondering about testing. […]
It is a good question although this change in of itself does nothing more than put all the preprocessor under the one symbol.
Regarding where to from here, my advice would be to make `emergency_help_message(void)` static internal to `flashrom.c` then follow up with changing both the signature's of the printer functions to `emergency_help_message(bool) && nonfatal_help_message(bool)` so they are simple printer functions then call them as follows `xxx_help_message(is_internal_programmer());`
Past that requires more substantive changes where possibly `is_internal_programmer()` is done inside `programmer_init()` and the resulting state is stored for later use in the flashctx or force_boardmismatch becomes part of programmer_entry. All this is a little unclear at the minute without some unrelated changes to surface the underlying hidden structures that best encode what we want.
Lastly, emulating 'broken hardware' is actually relatively easy. A EM100 could emulate a bad write op for a programmer which would exercise branches such as these. As discussed in the leadership meeting we would like to see a future where the CI bot runs precisely this kind of E2E test.
-
Edward O'Callaghan (Code Review)