Messages in this thread | | | Date | Mon, 20 Feb 2023 15:29:48 -0500 | From | "Vincent Dagonneau" <> | Subject | Re: [RFC PATCH 4/4] tools/nolibc: add tests for the integer limits in stdint.h |
| |
Hi Thomas,
On Sun, Feb 19, 2023, at 14:15, Willy Tarreau wrote: > Hi Thomas, > > On Sun, Feb 19, 2023 at 07:04:04PM +0000, Thomas Weißschuh wrote: >> > +#elif __SIZEOF_LONG__ == 4 >> > + 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; >> >> ptrdiff tests are duplicate. > > Argh, I thought I had already removed these duplicates, I noticed them > previously indeed. Vincent, please address this in your next iteration. >
Oops, my mistake, sorry about that. I removed it from v5.
>> > + CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffU); break; >> > +#else >> > +# warning "__SIZEOF_LONG__ is undefined" >> >> Why not #error? > > It's just a matter of choice. Since the tool's goal is to spot errors, > and if possible several at once, I find it preferable to still not fail > on other tests, as often when you get multiple failures it's easier to > figure what's going on. During the last test session I precisely had a > build error that was quite annoying because once I managed to fix it I > figured the fix was not the right one regarding other places. > > Alternately we could probably just add one line that always reports a > failure like the other ones (it would be even better so that we can > compare all outputs and still know that something fails): > > +#else > + CASE_TEST(__SIZEOF_LONG__defined); EXPECT_EQ(1, 1, 0); break; > >> > +#endif /* __WORDSIZE == 64 */ >> >> #endif comment is now incorrect > > Good catch indeed!
Thanks: fixed in v5.
>> >> > + case __LINE__: >> >> The "case" should be further left, no? > > You're right! > > Thank you! > Willy
Thank you for the review!
| |