Attention is currently required from: Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51925 )
Change subject: [WIP] replace programmer enum with a pointer to programmer_entry ......................................................................
Patch Set 4:
(5 comments)
This change is ready for review.
Patchset:
PS4: Looks quite good already. I think the DEFAULT_PROGRAMMER issue (and also the INTERNAL_PROGRAMMER checks) would solve itself if you would move the programmer_entry structs to the individual source files, first. e.g.
* first commit: move programmer_entry, turn programmer_table into an array of pointers (leaving the enum stuff as is for now), and s/programmer_table[.*]./programmer_table[.*]->/ everywhere.
* second commit: introduce a default_programmer pointer. (const, set at compile time, replacing CONFIG_DEFAULT_PROGRAMMER).
* third commit: this change, using `default_programmer` and `&internal_programmer` where needed.
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/51925/comment/de8b970a_e4e10051 PS4, Line 379: size_t _i Common flashrom style is to avoid such `for` local declarations. What I usually say here: the style should be discussed on the mailing list (and I really wouldn't mind).
Anyway, you can just use `i` here.
https://review.coreboot.org/c/flashrom/+/51925/comment/5e7672d3_809b9a64 PS4, Line 383: prog = &programmer_table[_i]; This needs to be moved to the positive cases below (':' and '\0'). Otherwise we would turn this into a prefix match by accident. Or, when we hit the `continue;` below, it should be reset to `NULL`.
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/51925/comment/f6a76ee5_276acf11 PS4, Line 2452: cb_check_image(newcontents, flash_size) < 0) { Should be indented with either 4 spaces or 2 tabs (shouldn't match with the body). Or don't break the line: our limit is 112 chars[1].
[1] https://flashrom.org/Development_Guidelines#Coding_style
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/51925/comment/5f85e505_4c9930cc PS4, Line 128: } This is quite weird. It leaves the last two pointers uninitialized. The API seems also meaningless, there is no way to know how many items are returned.