Attention is currently required from: Simon Buhrow, Thomas Heijligen, Aarya, Anastasia Klimchuk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67535 )
Change subject: [WIP] Unit tests for erase function selection algo ......................................................................
Patch Set 5:
(3 comments)
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/67535/comment/6900e556_f2bac54c PS5, Line 37: uint8_t buf[MOCK_CHIP_SIZE]; /* Contents of the mock chip. */ Can `buf` be moved into `struct all_state`? Then we just have one global buffer for the chip contents.
We'd also need to change `memcpy(buf, &g_state.test_cases[g_state.ind].buf[start], len);` to `memcpy(buf, &g_state.buf[start], len);` etc.
Then the test_case struct will only contain test parameters and can be const.
https://review.coreboot.org/c/flashrom/+/67535/comment/86b59401_6286a82b PS5, Line 183: ) We can reduce global state by passing the test case as an argument here, i.e.
`, const struct test_case *test_data)`
https://review.coreboot.org/c/flashrom/+/67535/comment/1be779c0_93153e0d PS5, Line 501: memcmp This should probably be`!memcmp(...`, otherwise `chip_erased` has the opposite meaning of the variable name. ditto in the functions below
-
Nikolai Artemiev (Code Review)