Attention is currently required from: Martin Roth, jacz@semihalf.com, Julius Werner, Jan Dabros. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49669 )
Change subject: tests: Add lib/region_file-test test case ......................................................................
Patch Set 3: Code-Review+2
(3 comments)
Patchset:
PS3: I have mostly stylistic questions left, one of them open (but feel free to disagree and resolve it in that case).
File tests/include/tests/lib/region_file_data.h:
https://review.coreboot.org/c/coreboot/+/49669/comment/ee3fb221_0022d769 PS3, Line 9: void array_dump(void *array, size_t length) I suppose this could/should be spun out? Can be done in a follow up commit, though
File tests/lib/region_file-test.c:
https://review.coreboot.org/c/coreboot/+/49669/comment/ba247245_a28a5d8a PS3, Line 307: setup_teardown_region_file_test, : setup_teardown_region_file_test), These means that the function is called twice between each test, right? I guess that's good for safety but also slightly excessive. Maybe add a comment to setup_teardown_region_file_test that the cost of memsetting everything twice is known but acceptable for testing?
-
Patrick Georgi (Code Review)