Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64385 )
Change subject: tests: add "stdlib.h" to the default includes ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
Patchset:
PS1:
It is used for malloc(), free(), etc. IMO using #includ <stdlib. […]
Oh no... this is not a replacement for `-includestdlib.h` in compile args.
These two entries in compile args: '-includestdlib.h', '-includeunittest_env.h' come together and they are part of the feature which enables memory checks in unit tests. What the feature is doing: checks there are no memory leaks at the end of test scenario.
These two includes *have to come first* in the list of all includes, and they *have to come exactly in this order*. This is why they are included via compile_args. This makes an effect on all flashrom source files (not only test files). The effect is that when flashrom is built for unit tests, functions like malloc(), free() etc are replaced with cmocka functions which keep track of allocated memory blocks. If the memory is allocated and not freed and the end of test scenario, test fails.
This comment applies the next patch as well (CB:64031). I will give it -1 so that it won't get merged accidentally. It looks to me this patch and the next kill the memory checks in tests. The tests still pass because the scenarios pass, but memory leaks are not detected anymore.
Hope you are not upset from -1, if we discuss and you convince me that memory checks still work, I will take it back!
How to verify whether feature works: for example comment `free(data)` in dummyflasher.c shutdown function and run tests. Tests for dummy should fail. If they pass, means the memory checks feature got killed.
For reference: feature was added in CB:51243
-
Anastasia Klimchuk (Code Review)