Attention is currently required from: Dmitry Zhadinets.
Anastasia Klimchuk has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/86921?usp=email )
Change subject: libflashrom: Added API to enumerate supported programmers ......................................................................
Patch Set 2:
(2 comments)
File tests/libflashrom.c:
https://review.coreboot.org/c/flashrom/+/86921/comment/a3bc684b_95243370?usp... : PS2, Line 22: enumerators_test_success I would rename this to `flashrom_supported_programmers_test_success` to use the same name as the function which is being tested. So that it's super obvious what the test is testing. (and in the next patch you will need to create a new test method, but I will comment there too)
https://review.coreboot.org/c/flashrom/+/86921/comment/f4005871_86c305a4?usp... : PS2, Line 24: (void) state; /* unused */ : const char** array = flashrom_supported_programmers(); : const char** ptr = array; : assert_non_null(array); : while (*(ptr++)){} : flashrom_data_free(array); : assert_int_not_equal(ptr - array, 0); This needs a fix for the indentation (use tabs always) Maybe you saw that already, but just in case, our code style is documented here: https://flashrom.org/dev_guide/development_guide.html#coding-style
Also, for readability, if you could add few more new lines: between 26-27, b/w 27-28, b/w 29-30
-
Anastasia Klimchuk (Code Review)