Name of user not set #1002873 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40538 )
Change subject: tests: Add lib/string-test test case ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40538/2/tests/lib/string-test.c File tests/lib/string-test.c:
https://review.coreboot.org/c/coreboot/+/40538/2/tests/lib/string-test.c@25 PS2, Line 25: /* Since strdup() requires malloc() it may only be invoked during ramstage : * phase. Expect dead_code/assert invocation otherwise. : */
nit: not actually removed?
Indeed, sorry. Some operational error here.
https://review.coreboot.org/c/coreboot/+/40538/2/tests/lib/string-test.c@34 PS2, Line 34: assert_int_equal(0, memcmp(str, duplicate, strlen(str)));
In that case, add a comment in this function that you're using assert_int_equal and memcmp instead of assert_string_equal because you don't want to use strcmp.
I've added a comment above this call. Makes sense, since this code will be used as an example, it's worth to mention that Cmocka is equipped with string-comparison functions.
Fair enough about assert_string_equal(). For strlen() you could use __builtin_strlen() (since it's a constant you could also just use (sizeof(str) - 1) in this case, but I think providing an example for using __builtin for this might be helpful). Guess we'll have to be a bit careful when testing libc stuff in general (thankfully, there's not too much of it in coreboot anyway).
Will use __builtin_strlen, good idea.
https://review.coreboot.org/c/coreboot/+/40538/3/tests/lib/string-test.c File tests/lib/string-test.c:
https://review.coreboot.org/c/coreboot/+/40538/3/tests/lib/string-test.c@13 PS3, Line 13: */
Maybe mention that __builtin_xxx can be used for many of the most simple mem*()/str*() functions if […]
Done