Feedback-ID: ic7094478:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-183-gbf7d00f500-fm-20230220.001-gbf7d00f5
Mime-Version: 1.0
Message-Id: <92950bb2-bca8-4278-87ea-d68c6687534d@app.fastmail.com>
In-Reply-To: <Y/nWwuqLLG8A0xT7@1wt.eu>
References: <20230223010025.11092-1-v@vda.io>
 <20230223010025.11092-5-v@vda.io> <Y/nWwuqLLG8A0xT7@1wt.eu>
Date:   Mon, 27 Feb 2023 21:10:20 -0500
From:   "Vincent Dagonneau" <v@vda.io>
To:     "Willy Tarreau" <w@1wt.eu>
Cc:     =?UTF-8?Q?Thomas_Wei=C3=9Fschuh?= <linux@weissschuh.net>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 4/4] tools/nolibc: add tests for the integer limits in stdint.h
Content-Type: text/plain
Precedence: bulk

On Sat, Feb 25, 2023, at 04:37, Willy Tarreau wrote:
> Hi Vincent,
>
> I'm currently integrating your patches. I'm having a quick question
> below:
>
> On Wed, Feb 22, 2023 at 08:00:25PM -0500, Vincent Dagonneau wrote:
>> This commit adds tests for the limits added in a previous commit. The
>> limits are defined in decimal in stdint.h and as hexadecimal in the
>> tests (e.g. 0x7f = 127 or 0x80 = -128). Hopefully it catches some of the
>> most egregious mistakes.
>> 
>> As we rely on the compiler to provide __SIZEOF_LONG__, we also test
>> whether it is defined.
>> 
>> Signed-off-by: Vincent Dagonneau <v@vda.io>
>> Signed-off-by: Willy Tarreau <w@1wt.eu>
>> ---
>>  tools/testing/selftests/nolibc/nolibc-test.c | 53 ++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>> 
>> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
>> index 882140508d56..d6886f900e79 100644
>> --- a/tools/testing/selftests/nolibc/nolibc-test.c
>> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
>> @@ -561,6 +561,59 @@ int run_syscall(int min, int max)
>>  		CASE_TEST(waitpid_child);     EXPECT_SYSER(1, waitpid(getpid(), &tmp, WNOHANG), -1, ECHILD); break;
>>  		CASE_TEST(write_badf);        EXPECT_SYSER(1, write(-1, &tmp, 1), -1, EBADF); break;
>>  		CASE_TEST(write_zero);        EXPECT_SYSZR(1, write(1, &tmp, 0)); break;
>> +		CASE_TEST(limit_int8_max);          EXPECT_EQ(1, INT8_MAX,         (int8_t)          0x7f); break;
>> +		CASE_TEST(limit_int8_min);          EXPECT_EQ(1, INT8_MIN,         (int8_t)          0x80); break;
>> +		CASE_TEST(limit_uint8_max);         EXPECT_EQ(1, UINT8_MAX,        (uint8_t)         0xff); break;
> (...)
>
> I'm just realizing now that the test was added at the end of the syscalls
> tests instead of the stdlib test (e.g. after memcmp()). Are you OK with me
> moving it there, given that it has nothing to do with syscalls but rather
> with what the nolibc itself provides ?
>

Hi, I was away for a few day. It is fine by me. If you want I'll just send another version with the Reviewed-By and the syscall -> stdlib move. 

Thank you again for all the review!
Vincent.

> Thanks!
> Willy