Attention is currently required from: Aarya, Edward O'Callaghan, Nikolai Artemiev, Simon Buhrow, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67535?usp=email )
Change subject: tests: Unit tests for erase function selection algo ......................................................................
Patch Set 12:
(7 comments)
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/67535/comment/0dd3ff16_b0c85758 : PS3, Line 238: void **state
`g_state` should probably be passed in here.
I will check cmocka docs how to do it, keep this unresolved for now.
https://review.coreboot.org/c/flashrom/+/67535/comment/980322b7_b84ad064 : PS3, Line 282: uint8_t *const newcontents = malloc(MIN_BUF_SIZE);
`uint8_t newcontents[MIN_BUF_SIZE];`
Done
https://review.coreboot.org/c/flashrom/+/67535/comment/4b751c86_d6323045 : PS3, Line 291: n
`&n`
Done
https://review.coreboot.org/c/flashrom/+/67535/comment/00fcf985_b2dfe629 : PS3, Line 294: n
`&n`
Done
https://review.coreboot.org/c/flashrom/+/67535/comment/c6e8a2a4_36d8898e : PS3, Line 313: : free(newcontents);
drops lines 313&&314.
Done
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/67535/comment/6a05e0ce_4789b28f : PS5, Line 37: uint8_t buf[MOCK_CHIP_SIZE]; /* Contents of the mock chip. */
Can buf be moved into struct all_state?
Yes! I did it.
But I wanted to clarify about:
Then the test_case struct will only contain test parameters and can be const.
What exactly do you mean?
https://review.coreboot.org/c/flashrom/+/67535/comment/4a3060f6_79358ac8 : PS5, Line 501: memcmp
This should probably be`!memcmp(... […]
Yes, makes sense. I changed so that now all three chip_erased eraseblocks_in_order eraseblocks_invocations mean what their names mean. However `!` has now moved further below, because I still need the final result to be 0 for success. But hope that's fine.
-
Anastasia Klimchuk (Code Review)