Attention is currently required from: Jakub Czapiga, Jan Dabros. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51804 )
Change subject: include/assert.h: Use mock_assert() for ENV_TEST targets ......................................................................
Patch Set 3:
(4 comments)
File src/include/assert.h:
https://review.coreboot.org/c/coreboot/+/51804/comment/5ee126c2_322ffb73 PS3, Line 12: const char *const file, const int line); nit: maybe declare this right next to MOCK_ASSERT() so it's more obvious where it belongs to?
https://review.coreboot.org/c/coreboot/+/51804/comment/b14dc855_5a014079 PS3, Line 38: mock_assert((result), (expression), (file), (line)) You can just use __ASSERT_FILE__ and __ASSERT_LINE__ in here, no need to pass so many things.
https://review.coreboot.org/c/coreboot/+/51804/comment/2d4854ab_65b5edf1 PS3, Line 71: MOCK_ASSERT(0, "BUG ENCOUNTERED", \ Bad indentation.
File src/include/assert.h:
https://review.coreboot.org/c/coreboot/+/51804/comment/32f4963a_525afcf7 PS1, Line 67: #define assert(statement) mock_assert(statement, #statement, __FILE__, __LINE__)
ASSERT(), ASSERT_MSG() and BUG() have different input parameters. […]
I mean you could do something like
#define HANDLE_ASSERT(reason, message) do { if (ENV_TEST) mock_assert(result, message, __ASSERT_FILE__, __ASSERT_LINE__); if (CONFIG(FATAL_ASSERTS)) hlt();
to deduplicate things a bit more. But I'm fine with what you have right now as well.
-
Julius Werner (Code Review)