Attention is currently required from: David Hendricks, Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51243 )
Change subject: Enable dynamic memory allocation checks for cmocka unit tests ......................................................................
Patch Set 2: Code-Review+1
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/51243/comment/dce00df4_ebcd51e1 PS2, Line 10: files and tests. Nit. If these are paragraphs, split them with empty lines. If they aren't, don't wrap the lines after the full stop.
Patchset:
PS2: The preprocessor solution to override stdlib functions looks a bit fragile. If an inline function in another header file would use malloc(), would that still be caught? There may be a way out by always including `stdlib.h` first and then `unittest_env.h` before anything else. That would be easy from the command line with `-include` directives. And it wouldn't need `UNIT_TEST_ENV`.
File tests/flashrom.c:
https://review.coreboot.org/c/flashrom/+/51243/comment/e435f40d_3c35633e PS2, Line 27: #define assert_string_not_equal_with_free(text, expected) \ NB. I known this is probably just following cmocka, but I find it very hard to visually distinguish the two
assert_string_equal_with_free
assert_string_not_equal_with_free
The longer the boilerplate around the *not* becomes, the harder it is to "see" it (without fully reading the identifier).