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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y/J1RB1T4tFeBxgs@1wt.eu>
Date:   Sun, 19 Feb 2023 20:15:16 +0100
From:   Willy Tarreau <w@....eu>
To:     Thomas Weißschuh <thomas@...ch.de>
Cc:     Vincent Dagonneau <v@....io>, linux-kernel@...r.kernel.org
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 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