Messages in this thread | | | Date | Sun, 12 Feb 2023 11:41:15 +0100 | From | Willy Tarreau <> | Subject | Re: [PATCH v4 0/4] tools/nolibc: Adding stdint.h, more integer types and tests |
| |
Hi Vincent,
On Fri, Feb 10, 2023 at 08:03:02AM -0500, Vincent Dagonneau wrote: > > Thanks Vincent. At first glance it looks good. I'll give it a try on > > all supported archs to make sure we didn't overlook anything and we'll > > merge it. One tiny comment though, look below: > > > >> Vincent Dagonneau (4): > >> tools/nolibc: Adding stdint.h > >> tools/nolibc: Adding integer types and integer limit macros > >> tools/nolibc: Enlarging column width of tests > >> tools/nolibc: Adds tests for the integer limits in stdint.h > > > > I mentioned in the first review that it's generally preferred to use > > the imperative form rather than present participle on subject lines. > > This would give: > > > > tools/nolibc: Add stdint.h > > tools/nolibc: Add integer types and integer limit macros > > tools/nolibc: Enlarge column width of tests > > tools/nolibc: Add tests for the integer limits in stdint.h > > > > I can perform this trivial change locally before merging without asking > > you to respin a series just for this if that's OK for you. Just let me > > know. > > > > Thanks! > > Willy > > Yep, go ahead!
Done, however I'm seeing the following failures on aarch64/riscv64/s390x:
$ grep -B 200 limit.*FAIL stdint.out | grep '\(limit.*FAIL\|gcc-11.3.0-nolibc.*-O0\)' /f/tc/nolibc/gcc-11.3.0-nolibc/aarch64*/bin/aarch64*-gcc -g -O0 -g -o nolibc-test \ 100 limit_intptr_min = -2147483648 [FAIL] 103 limit_ptrdiff_min = -2147483648 [FAIL] 105 limit_ptrdiff_min = -2147483648 [FAIL] /f/tc/nolibc/gcc-11.3.0-nolibc/riscv64*/bin/riscv64*-gcc -g -O0 -g -o nolibc-test \ 100 limit_intptr_min = -2147483648 [FAIL] 103 limit_ptrdiff_min = -2147483648 [FAIL] 105 limit_ptrdiff_min = -2147483648 [FAIL] /f/tc/nolibc/gcc-11.3.0-nolibc/s390*/bin/s390*-gcc -g -march=z10 -m64 -O0 -g -o nolibc-test \ 100 limit_intptr_min = -2147483648 [FAIL] 103 limit_ptrdiff_min = -2147483648 [FAIL] 105 limit_ptrdiff_min = -2147483648 [FAIL]
It makes me think that the __WORDSIZE==64 condition didn't match there, I'm investigating. However while looking at this I noticed a mistake in your patch: in the 32-bit part, limit_ptrdiff_{min,max} were repeated, and no least64_{min,max} tests were placed, so I sense a copy-paste mistake though I'm uncertain about the initial intent. If you just want me to drop the duplicate lines I can easily do it, just let me know. I'll be back with more info once I figure the reason for these archs not using __WORDSIZE==64.
#if __WORDSIZE == 64 CASE_TEST(limit_int_least64_max); EXPECT_EQ(1, INT_LEAST64_MAX, (int_least64_t) 0x7fffffffffffffffLL); break; CASE_TEST(limit_int_least64_min); EXPECT_EQ(1, INT_LEAST64_MIN, (int_least64_t) 0x8000000000000000LL); break; CASE_TEST(limit_uint_least64_max); EXPECT_EQ(1, UINT_LEAST64_MAX, (uint_least64_t) 0xffffffffffffffffULL); break; CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x8000000000000000LL); break; CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffffffffffffLL); break; CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffffffffffULL); break; CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x8000000000000000LL); break; CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffffffffffffLL); break; CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffffffffffULL); break; #else CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x80000000); break; CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffff); break; CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffU); break; CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break; CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break; CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break; CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break; CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffU); break; #endif /* __WORDSIZE == 64 */
Regards, Willy
| |