Messages in this thread | | | Date | Mon, 06 Feb 2023 17:19:18 -0500 | From | "Vincent Dagonneau" <> | Subject | Re: [PATCH v3 0/4] tools/nolibc: Adding stdint.h, more integer types and tests |
| |
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.
| |