michaelh/zephyr
1
0
Fork 0
Code Releases Activity

lib/os/cbprintf_nano.c: avoid sign extension on unsigned formats

Browse source 
There might be a sign extension when a long is promoted to
int_value_type and the former type is smaller than the later.
This produces the wrong output if the specified format is unsigned.

Let's avoid this problem by handling signed and unsigned cases
explicitly. When the type already matches int_value_type then the
compiler is smart enough to recognize the redundancy and removes
unneeded duplications automatically, meaning that the code will stay
small when code size matters.

A similar issue also existed in the restricted %llu case.
The fix is the same as above.

Those fixes exposed wrong results in the printk.c test with %llx
so fix that as well.

Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
This commit is contained in:
Nicolas Pitre 2021-08-31 16:05:18 -04:00 • committed by Anas Nashif
commit 5a384b9ea8
3 changed files with 52 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

57
lib/os/cbprintf_nano.c
View file

@ -174,20 +174,40 @@ start:
uint_value_type d;
if (length_mod == 'z') {
d = va_arg(ap, ssize_t);
} else if (length_mod == 'l') {
d = va_arg(ap, long);
} else if (length_mod == 'L') {
long long lld = va_arg(ap, long long);
if (sizeof(int_value_type) < 8U &&
lld != (int_value_type) lld) {
data = "ERR";
data_len = 3;
precision = 0;
break;
if (*fmt == 'u') {
d = va_arg(ap, size_t);
} else {
d = va_arg(ap, ssize_t);
}
} else if (length_mod == 'l') {
if (*fmt == 'u') {
d = va_arg(ap, unsigned long);
} else {
d = va_arg(ap, long);
}
} else if (length_mod == 'L') {
if (*fmt == 'u') {
unsigned long long llu =
va_arg(ap, unsigned long long);
if (llu != (uint_value_type) llu) {
data = "ERR";
data_len = 3;
precision = 0;
break;
}
d = (uint_value_type) llu;
} else {
long long lld = va_arg(ap, long long);
if (lld != (int_value_type) lld) {
data = "ERR";
data_len = 3;
precision = 0;
break;
}
d = (int_value_type) lld;
}
d = (uint_value_type) lld;
} else if (*fmt == 'u') {
d = va_arg(ap, unsigned int);
} else {
@ -229,7 +249,16 @@ start:
} else if (length_mod == 'l') {
x = va_arg(ap, unsigned long);
} else if (length_mod == 'L') {
x = va_arg(ap, unsigned long long);
unsigned long long llx =
va_arg(ap, unsigned long long);
if (llx != (uint_value_type) llx) {
data = "ERR";
data_len = 3;
precision = 0;
break;
}
x = (uint_value_type) llx;
} else {
x = va_arg(ap, unsigned int);
}

2
tests/kernel/common/src/printk.c
View file

@ -51,7 +51,7 @@ char *expected = "22 113 10000 32768 40000 22\n"
"42 42 42 42\n"
"42 42 0042 00000042\n"
"255 42 abcdef 42\n"
"ERR -1 4294967295 ffffffff\n"
"ERR -1 ERR ERR\n"
"0xcafebabe 0xbeef 0x2a\n"
;
#endif

15
tests/unit/cbprintf/main.c
View file

@ -510,6 +510,7 @@ static void test_d_length(void)
int max = 1876543210;
long long svll = 123LL << 48;
long long svll2 = -2LL;
unsigned long long uvll = 4000000000LLU;
int rc;
TEST_PRF(&rc, "%d/%d", min, max);
@ -526,22 +527,22 @@ static void test_d_length(void)
PRF_CHECK("46/-22", rc);
}
TEST_PRF(&rc, "%ld/%ld", (long)min, (long)max);
TEST_PRF(&rc, "%ld/%ld/%lu/", (long)min, (long)max, 4000000000UL);
if (IS_ENABLED(CONFIG_CBPRINTF_FULL_INTEGRAL)
|| (sizeof(long) <= 4)
|| IS_ENABLED(CONFIG_CBPRINTF_NANO)) {
PRF_CHECK("-1234567890/1876543210", rc);
PRF_CHECK("-1234567890/1876543210/4000000000/", rc);
} else {
PRF_CHECK("%ld/%ld", rc);
PRF_CHECK("%ld/%ld/%lu/", rc);
}
TEST_PRF(&rc, "/%lld/%lld/%lld/", svll, -svll, svll2);
TEST_PRF(&rc, "/%lld/%lld/%lld/%llu/", svll, -svll, svll2, uvll);
if (IS_ENABLED(CONFIG_CBPRINTF_FULL_INTEGRAL)) {
PRF_CHECK("/34621422135410688/-34621422135410688/-2/", rc);
PRF_CHECK("/34621422135410688/-34621422135410688/-2/4000000000/", rc);
} else if (IS_ENABLED(CONFIG_CBPRINTF_COMPLETE)) {
PRF_CHECK("/%lld/%lld/%lld/", rc);
PRF_CHECK("/%lld/%lld/%lld/%llu/", rc);
} else if (IS_ENABLED(CONFIG_CBPRINTF_NANO)) {
PRF_CHECK("/ERR/ERR/-2/", rc);
PRF_CHECK("/ERR/ERR/-2/4000000000/", rc);
} else {
zassert_true(false, "Missed case!");
}