Hi, Thomas, Willy and David
Hi all,
On Sun, Aug 27, 2023 at 11:17:19AM +0200, Thomas Wei�schuh wrote:
To be honest I don't see a problem with the current aproach. It is very obvious what is going on, the same pattern is used by other projects and the "overhead" is very small.
It seems the macros will only work for simple cases which only test the availability of a single syscall number.
Good news, as I just replied [1] and as the test [2] shows, the __stringify() based __is_nr_defined() macro (proposed in this RFC thread) can test syscall definition for us, and the __stringify() and __nrtoi() macro (Derive from David's NR_toi() work [3],[4]) based __get_nr() macro can get the syscall number constant, both of them are compiling stage cost and the compiling is not slow for all of the current supported architectures, no size cost no runtime cost and even help us to get smaller binary ;-)
[1]: https://lore.kernel.org/lkml/20230829233912.63097-1-falcon@tinylab.org/ [2]: https://godbolt.org/z/a7hxWj83E [3]: https://lore.kernel.org/lkml/6819b8e273dc44e18f14be148549b828@AcuMS.aculab.c... [4]: https://godbolt.org/z/rear4c1hj
Of these we currently only have: gettimeofday(), lseek(), statx(), wait4()
So in it's current form we save 4 * 4 = 16 lines of code. The proposed solution introduces 14 + 2 (empty) = 16 lines of new code, and a bunch of mental overhead.
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 690
Including 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
In case multiple underlying syscalls can be used these take different arguments which a simple macro won't be able to encode sanely.
I totally agree, I would prefer all this to be manageable by humans with no preprocessor brain implant as much as possible as well.
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? ;-)
Thanks, Zhangjin
Thanks, Willy