On Wed, Aug 30, 2023 at 08:19:07AM +0800, Zhangjin Wu wrote:
Yes, as also suggested by Willy, the old proposed method redefined NOLIBC__NR_* macros for every __NR_* and it must be avoided, and now, the __is_nr_defined() and __get_nr() macros will simply avoid defining new NOLIBC__NR_* for exisitng __NR_*, they can be used to test and get the existing __NR_* directly.
In my local repo, we have saved 500+ lines ;-)
$ git show nolibc/next:tools/include/nolibc/sys.h | wc -l 1190 $ cat tools/include/nolibc/sys.h | wc -l 690Including all of the -ENOSYS and #ifdef's:
$ git grep -r ENOSYS nolibc/next:tools/include/nolibc/sys.h | wc -l 17 $ git grep -Er "#ifdef|#el|#endif" nolibc/next:tools/include/nolibc/sys.h | wc -l 77
And how many hacks or bugs for the rare special cases ? I'm not kidding, this obsession for removing lines has already caused us quite some trouble around sysret() in the previous cycle, and I yet have to see the gain for maintenance.
I do have comparable macros that I never employed in my projects just because each time I needed them I found a corner case at one particular optimization level or with a particular compiler version where you manage to break them, and suddenly all the world falls apart. I'm fine for taking that risk when there is a *real* benefit, but here we're speaking about replacing existing, readable and auditable code by something more compact that becomes completely unauditable. I could understand that if it was a specific required step in a more long-term project of factorizing something, but there still hasn't been any such project defined, so all we're doing is "let's see if we can do this or that and see if it looks better". I continue to strongly disagree with this approach, it causes all of us a lot of extra work, introduces regressions and nobody sees the benefits in the end.
Instead of using tricks here and there to remove lines, I'd rather have an approach centered on the code's architecture and modularity to see what are the current problems and how they should be addressed.
For now I still find it complicated to explain other maintainers how to test their changes on all architectures. I've found it difficult to switch between arm and thumb modes for arm when trying to explain it lately (now with more analysis I'm seeing that I could have placed it into CFLAGS_arm for example) so it means we're missing some doc in the makefile itself or on the usage in general. I've faced the problem you met with some builds failing on "you need to run mrproper first", which makes me think that in fact it should probably be "make defconfig" or "make prepare" that does this. Just checking the makefile and that's already the case, yet I faced the issue, so maybe it's caused by -j being passed through all the chain and we need to serialize the operations, I don't know.
I would also like that we clarify some use cases. Originally the project started as the single-file zero-installation file that allowed to build static binaries, retrieving the linux/ subdir from wherever it was (i.e. from the local system libc for native builds or from the toolchain used for cross-builds). Since we've started to focus a bit too much on the nolibc-test program only with its preparation stages, I think we've lost this focus a little bit, and I'd like to add some tests to make sure this continues to work (I know that my primary usage already got broken by the statx change with the toolchain I was using).
Also, maybe it could be useful to make it easier to produce tar.gz sysroots form tools/include/nolibc for various (even all?) archs to make it easier for users to test their own code against it.
So in short, we need to make it easier to use and easier to test, not to just remove lines that nobody needs to maintain.
Yeah, for the sys_* definitions, it is ok for us to use explicit arguments intead of the '...'/__VA_ARGS__ to avoid losing some arguments sometimes, let's do it in the RFC patchset but it should come after the tinyconfig patchset.
BTW, Willy, when will you prepare the branch for v6.7 developmlent? ;-)
You can continue to use the latest branch as a starting point, we'll create the new for-6.7 branch once 6.6-rc1 is out.
Thanks, Willy