Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73039 )
Change subject: flashrom: rewrite flashbuses_to_text() ......................................................................
Patch Set 4:
(4 comments)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/73039/comment/0b55dc0d_59cfeab5 PS4, Line 918: Get You can leave "return" in this case, I think return sounds good here.
https://review.coreboot.org/c/flashrom/+/73039/comment/a8ffe165_edecbaa7 PS4, Line 924: char *ret, *ptr; I am going to ask silly question: why do you need both? If there is a reason, I think it can be a comment.
https://review.coreboot.org/c/flashrom/+/73039/comment/d4482b8d_7964f041 PS4, Line 935: Once all chipsets and flash chips have been updated An open question to everyone who reads this: what does it even mean? :) Specifically, I am wondering what should happen so that this fixme can be fixed.
https://review.coreboot.org/c/flashrom/+/73039/comment/fae9aaca_19543361 PS4, Line 938: if (bustype == BUS_NONSPI) : return strdup("Non-SPI"); : if (bustype == BUS_NONE) : return strdup(bustypes[bustype]); These first two cases (BUS_NONSPI and BUS_NONE) are specially handled in the code, I understand why, but what looks a bit odd is that the former is hard-coded, the latter is taken from an array. Why the difference? Do you need BUS_NONE is the array? Maybe make it hard-coded too, and remove from array?
-
Anastasia Klimchuk (Code Review)