Re: [RFC PATCH 4/4] tools/nolibc: add tests for the integer limits in stdint.h

[Willy Tarreau]

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.

> > +		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!
> 
> > +			case __LINE__:
> 
> The "case" should be further left, no?

You're right!

Thank you!
Willy