Feedback-ID: ic7094478:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-108-ge995779fee-fm-20230203.001-ge995779f
Mime-Version: 1.0
Message-Id: <bff2a9f8-6f70-409a-8acb-cf8917f653a6@app.fastmail.com>
In-Reply-To: <Y+C6mj4F77U43atS@1wt.eu>
References: <20230206013248.471664-1-v@vda.io> <Y+C6mj4F77U43atS@1wt.eu>
Date:   Mon, 06 Feb 2023 17:19:18 -0500
From:   "Vincent Dagonneau" <v@vda.io>
To:     "Willy Tarreau" <w@1wt.eu>
Cc:     linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 0/4] tools/nolibc: Adding stdint.h, more integer types and tests
Content-Type: text/plain
Precedence: bulk

On Mon, Feb 6, 2023, at 03:30, Willy Tarreau wrote:
> Hi Vincent,
>
> On Sun, Feb 05, 2023 at 08:32:45PM -0500, Vincent Dagonneau wrote:
>> Hi,
>> 
>> This is version 3 of my patch to add stdint.h to nolibc. Previous
>> versions of this patch are available here:
>> 
>> * v2: https://lore.kernel.org/all/20230202201101.43160-1-v@vda.io/
>> * v1: https://lore.kernel.org/all/20230202160236.25342-1-v@vda.io/
>> 
>> This version should address most of the concerns raised by Willy on the
>> previous version (missing LL, ULL suffixes, uintptr/size_max size).
>
> OK thank you, that's much better already. I'm still having a few comments:
>   - please avoid the present participle in the subjects of your commits,
>     the imperative form is generally preferred (e.g: "add foo" instead of
>     "adding foo").
>

Noted.

>   - do not forget your signed-off-by on patches.
>

Again, sorry. I'm quite new to the send-email workflow. I just figured that I was probably supposed to use format-patch then send-email rather than just send-email... Makes way more sense though and now I can have a file with my cover letter and carry it over to the next version.

>   - 3rd commit had an empty message. There's always something to say
>     about a change, at least why and what it provides.
>

Noted.

>   - You still have this definition which is only valid for 64-bit,
>     you will need to move it to your ifdef __WORDSIZE block to adjust
>     it based on the word size:
>
>     #define         SIZE_MAX UINT64_MAX
>
>     Maybe this one should also be part of the tests ?
>

Ok.

>> I tested it successfully on x86_64 and arm64 (on qemu). I have some
>> trouble setting up my cross compilation chain for i386, arm and mips
>> however.
>
> What compilers do you use ? I'm using version 11.3 from these ones:
>
>   https://mirrors.edge.kernel.org/pub/tools/crosstool/
>
> All supported archs work fine for me. Usually on x86_64 you can even
> start the i386 binaries locally (if your kernel has COMPAT or COMPAT_32
> built in, which is most often the case). And very commonly arm64 support
> armv7 binaries as well.
> 
> Just let me know if I can help you set up your environment, it's always
> a good long-term investment!
>

Ah, foolishly I was relying on the packaged cross compiler for Arch. Compiled everything just fine with the packaged versions on Ubuntu (gcc-10 mips/arm/arm64/i386). I'll try with the toolchains @ https://mirrors.edge.kernel.org/pub/tools/crosstool/ as well.

> Regards,
> Willy

Thanks,
Vincent.