Messages in this thread | | | Date | Mon, 6 Feb 2023 09:30:18 +0100 | From | Willy Tarreau <> | Subject | Re: [PATCH v3 0/4] tools/nolibc: Adding stdint.h, more integer types and tests |
| |
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").
- do not forget your signed-off-by on patches.
- 3rd commit had an empty message. There's always something to say about a change, at least why and what it provides.
- 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 ?
> 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!
Regards, Willy
| |