Fix format specifiers zassert strings #96906
Conversation
ZPOW2_CEIL returns unsigned long, so use it to store result and use same format specifier for actual and expected values. Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
pdgendt
added
the
Hotfix
zephyrbot
added
area: C Library
zephyrbot
requested review from
TaiJuWu,
aescolar,
andyross,
ceolin,
cfriedt,
dcpleung,
jukkar,
keith-packard,
nashif,
npitre,
pdgendt,
peter-mitsis,
rlubos,
ssharks and
stephanosio
Status is of type int, not long long unsigned int, so use %d instead of %llu. Use %zu for size_t. Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
param is of type uintptr_t, so use %lu for printing it. Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
Remove the second parameter of zassert_ok() calls, as it internally checks for equality with 0. The second parameter should be the error message. Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
Correctly escape percent sign in zassert strings. Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
tpambor
dismissed stale reviews from jhedberg, henrikbrixandersen, and pdgendt
via
94d8acc
|
tpambor
removed
the
Hotfix
| zassert_equal(thrd_success, thrd_create(&thr, fun, ¶m)); | ||
| zassert_equal(thrd_success, thrd_join(thr, &res)); | ||
| zassert_equal(BIOS_FOOD, param, "expected: %d actual: %d", BIOS_FOOD, param); | ||
| zassert_equal(BIOS_FOOD, param, "expected: %lu actual: %lu", BIOS_FOOD, param); |
There was a problem hiding this comment.
param is uintptr_t, which is not a type that printf knows about internally. Include <inttypes.h> and use "%" PRIuPTR for both, adding a cast to (uintptr_t) for BIOS_FOOD so they match.
There was a problem hiding this comment.
I didn't use PRIuPTR or others from inttypes.h, as they are not widely used in Zephyr. A quick search showed only three occurrences of PRIuPTR. As far as I know, in Zephyr's type system, PRIuPTR is always lu.
So int32_t is always an int, intptr_t is always a long, and PRIuPTR is
always "lu", for all supported architectures.
If there is consensus to start using defines from inttypes.h, I'm happy to use them.
There was a problem hiding this comment.
Several people have commented that the inttypes.h stuff is not their favorite, to the point that Zephyr attempts to enforce some level of type uniformity. It usually overrides the toolchain types by pre-loading zephyr_stdint.h. you can use %lu with uintptr_t and %d with int32_t when CONFIG_ENFORCE_ZEPHYR_STDINT is set.
There was a problem hiding this comment.
I guess a safe option would be to cast both parameters to (unsigned long) and use "%ul" (this is a error print at the end)
There was a problem hiding this comment.
yeah, a reasonable plan (although, I think you meant %lu)
There was a problem hiding this comment.
👍 I used now the cast + lu, see #97202
|
|
||
| /* arbitrary magic numbers used for testing */ | ||
| #define BIOS_FOOD 0xb105f00d | ||
| #define BIOS_FOOD 0xb105f00dUL |
There was a problem hiding this comment.
I'm not sure what the best way to represent a uintptr_t constant is -- C has things like UINT64_C(x), which can be used to create a 64-bit constant, but it doesn't have UINTPTR_C(x) for uintptr_t. An odd asymmetry in the C spec. Probably the most spec-compliant way to define this would be ((uintptr_t) UINTMAX_C(0xb105f00d))
|
I will split this up into multiple PRs, so the changes can be easier reviewed be the respective maintainers. I created a tracking issue here #96968. |
|
I will close this PR. I have now submitted individual PRs for all issues, for details see #96968. |
10 participants
For details see individual commits.
Fixes errors that were found after adding validation of zassert string in #96335.