Willy, Thomas
This is the revision of the v2 syscall helpers [1], it is based on
20230606-nolibc-rv32+stkp7a of [2]. It doesn't conflict with the v4 of
-ENOSYS patchset [3], so, it is ok to simply merge both of them.
This revision mainly applied Thomas' method, removed the __syscall()
helper and replaced it with __sysret() instead, because __syscall()
looks like _syscall() and syscall(), it may mixlead the developers.
Changes from v2 -> v3:
* tools/nolibc: sys.h: add a syscall return helper
* The __syscall() is removed.
* Align the code style of __sysret() with the others, and use
__inline__ instead of inline (like stdlib.h) to let it work with
the default -std=c89 in tools/testing/selftests/nolibc/Makefile
* tools/nolibc: unistd.h: apply __sysret() helper
As v2.
* tools/nolibc: sys.h: apply __sysret() helper
replaced __syscall() with __sysret() and merged two separated patches of v2 to one.
Did run-user tests for rv32 (with [3]), rv64 and arm64.
BTW, two questions for Thomas,
* This commit 659a49abc9c2 ("tools/nolibc: validate C89 compatibility")
enables -std=c89, why not gnu11 used by kernel ? ;-)
* Do we need to tune the order of the macros in unistd.h like this:
#define _syscall(N, ...) __sysret(my_syscall##N(__VA_ARGS__))
#define _syscall_n(N, ...) _syscall(N, __VA_ARGS__)
#define __syscall_narg(_0, _1, _2, _3, _4, _5, _6, N, ...) N
#define _sycall_narg(...) __syscall_narg(__VA_ARGS__, 6, 5, 4, 3, 2, 1, 0)
#define syscall(...) _syscall_n(_sycall_narg(__VA_ARGS__), ##__VA_ARGS__)
Before, It works but seems not put in using order:
#define _syscall(N, ...) __sysret(my_syscall##N(__VA_ARGS__))
#define _sycall_narg(...) __syscall_narg(__VA_ARGS__, 6, 5, 4, 3, 2, 1, 0)
#define __syscall_narg(_0, _1, _2, _3, _4, _5, _6, N, ...) N
#define _syscall_n(N, ...) _syscall(N, __VA_ARGS__)
#define syscall(...) _syscall_n(_sycall_narg(__VA_ARGS__), ##__VA_ARGS__)
Thanks.
Best regards,
Zhangjin
---
[1]: https://lore.kernel.org/linux-riscv/[email protected]/
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git
[3]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
Zhangjin Wu (3):
tools/nolibc: sys.h: add a syscall return helper
tools/nolibc: unistd.h: apply __sysret() helper
tools/nolibc: sys.h: apply __sysret() helper
tools/include/nolibc/sys.h | 364 +++++-----------------------------
tools/include/nolibc/unistd.h | 11 +-
2 files changed, 55 insertions(+), 320 deletions(-)
--
2.25.1
Use __sysret() to shrink the whole _syscall() to oneline code.
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/include/nolibc/unistd.h | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/tools/include/nolibc/unistd.h b/tools/include/nolibc/unistd.h
index c20b2fbf065e..7e3c005d28ba 100644
--- a/tools/include/nolibc/unistd.h
+++ b/tools/include/nolibc/unistd.h
@@ -56,16 +56,7 @@ int tcsetpgrp(int fd, pid_t pid)
return ioctl(fd, TIOCSPGRP, &pid);
}
-#define _syscall(N, ...) \
-({ \
- long _ret = my_syscall##N(__VA_ARGS__); \
- if (_ret < 0) { \
- SET_ERRNO(-_ret); \
- _ret = -1; \
- } \
- _ret; \
-})
-
+#define _syscall(N, ...) __sysret(my_syscall##N(__VA_ARGS__))
#define _sycall_narg(...) __syscall_narg(__VA_ARGS__, 6, 5, 4, 3, 2, 1, 0)
#define __syscall_narg(_0, _1, _2, _3, _4, _5, _6, N, ...) N
#define _syscall_n(N, ...) _syscall(N, __VA_ARGS__)
--
2.25.1
On 2023-06-07 19:28:58+0800, Zhangjin Wu wrote:
> Willy, Thomas
>
> This is the revision of the v2 syscall helpers [1], it is based on
> 20230606-nolibc-rv32+stkp7a of [2]. It doesn't conflict with the v4 of
> -ENOSYS patchset [3], so, it is ok to simply merge both of them.
>
> This revision mainly applied Thomas' method, removed the __syscall()
> helper and replaced it with __sysret() instead, because __syscall()
> looks like _syscall() and syscall(), it may mixlead the developers.
>
> Changes from v2 -> v3:
>
> * tools/nolibc: sys.h: add a syscall return helper
>
> * The __syscall() is removed.
>
> * Align the code style of __sysret() with the others, and use
> __inline__ instead of inline (like stdlib.h) to let it work with
> the default -std=c89 in tools/testing/selftests/nolibc/Makefile
>
> * tools/nolibc: unistd.h: apply __sysret() helper
>
> As v2.
>
> * tools/nolibc: sys.h: apply __sysret() helper
>
> replaced __syscall() with __sysret() and merged two separated patches of v2 to one.
>
> Did run-user tests for rv32 (with [3]), rv64 and arm64.
>
> BTW, two questions for Thomas,
>
> * This commit 659a49abc9c2 ("tools/nolibc: validate C89 compatibility")
> enables -std=c89, why not gnu11 used by kernel ? ;-)
Because nolibc needs to support whatever its users need.
As nolibc is header-only all of it needs to work everywhere.
C89 should work for everybody :-)
The kernel on the other hand is compiled standalone and is not limited
by its users.
See the discussion here:
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/
> * Do we need to tune the order of the macros in unistd.h like this:
>
> #define _syscall(N, ...) __sysret(my_syscall##N(__VA_ARGS__))
> #define _syscall_n(N, ...) _syscall(N, __VA_ARGS__)
> #define __syscall_narg(_0, _1, _2, _3, _4, _5, _6, N, ...) N
> #define _sycall_narg(...) __syscall_narg(__VA_ARGS__, 6, 5, 4, 3, 2, 1, 0)
> #define syscall(...) _syscall_n(_sycall_narg(__VA_ARGS__), ##__VA_ARGS__)
>
> Before, It works but seems not put in using order:
>
> #define _syscall(N, ...) __sysret(my_syscall##N(__VA_ARGS__))
> #define _sycall_narg(...) __syscall_narg(__VA_ARGS__, 6, 5, 4, 3, 2, 1, 0)
> #define __syscall_narg(_0, _1, _2, _3, _4, _5, _6, N, ...) N
> #define _syscall_n(N, ...) _syscall(N, __VA_ARGS__)
> #define syscall(...) _syscall_n(_sycall_narg(__VA_ARGS__), ##__VA_ARGS__)
Not sure it makes a big difference.
If you want to change it, go for it.
> Thanks.
>
> Best regards,
> Zhangjin
>
> ---
> [1]: https://lore.kernel.org/linux-riscv/[email protected]/
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git
> [3]: https://lore.kernel.org/linux-riscv/[email protected]/T/#t
>
> Zhangjin Wu (3):
> tools/nolibc: sys.h: add a syscall return helper
> tools/nolibc: unistd.h: apply __sysret() helper
> tools/nolibc: sys.h: apply __sysret() helper
>
> tools/include/nolibc/sys.h | 364 +++++-----------------------------
> tools/include/nolibc/unistd.h | 11 +-
> 2 files changed, 55 insertions(+), 320 deletions(-)
For the full series:
Reviewed-by: Thomas Weißschuh <[email protected]>
Thanks,
Thomas
> On 2023-06-07 19:28:58+0800, Zhangjin Wu wrote:
> > Willy, Thomas
> >
> > This is the revision of the v2 syscall helpers [1], it is based on
> > 20230606-nolibc-rv32+stkp7a of [2]. It doesn't conflict with the v4 of
> > -ENOSYS patchset [3], so, it is ok to simply merge both of them.
> >
> > This revision mainly applied Thomas' method, removed the __syscall()
> > helper and replaced it with __sysret() instead, because __syscall()
> > looks like _syscall() and syscall(), it may mixlead the developers.
> >
(...)
> > BTW, two questions for Thomas,
> >
> > * This commit 659a49abc9c2 ("tools/nolibc: validate C89 compatibility")
> > enables -std=c89, why not gnu11 used by kernel ? ;-)
>
> Because nolibc needs to support whatever its users need.
> As nolibc is header-only all of it needs to work everywhere.
> C89 should work for everybody :-)
>
Get it, thanks.
> The kernel on the other hand is compiled standalone and is not limited
> by its users.
>
> See the discussion here:
>
> https://lore.kernel.org/all/[email protected]/
> https://lore.kernel.org/all/[email protected]/
>
Thanks very much for sharing the whole history info.
And as the your commit 063b6bc5b39f ("tools/nolibc: use __inline__ syntax")
explains, the 'inline' keyword has been used in many headers of include/uapi/,
so, how our -std=c89 work with them? I did find the clue eventually, here maybe:
$ grep -n inline scripts/headers_install.sh
11: echo "asm/inline/volatile keywords."
37: s/(^|[[:space:](])(inline|asm|volatile)([[:space:](]|$)/\1__\2__\3/g
The headers_install target helped us convert all of the new keywords to the old
ones, it's magic ;-)
So, it should work if people not want to try a -I/path/to/include/uapi/, I did
this for musl before, even If we do this, this may help:
diff --git a/tools/include/nolibc/std.h b/tools/include/nolibc/std.h
index 933bc0be7e1c..33d546cf9af0 100644
--- a/tools/include/nolibc/std.h
+++ b/tools/include/nolibc/std.h
@@ -7,6 +7,14 @@
#ifndef _NOLIBC_STD_H
#define _NOLIBC_STD_H
+#ifndef NOLIBC_TEST
+#ifndef __STDC_VERSION__
+#define inline __inline__
+#define asm __asm__
+#define volatile __volatile__
+#endif
+#endif
+
/* Declare a few quite common macros and types that usually are in stdlib.h,
* stdint.h, ctype.h, unistd.h and a few other common locations. Please place
* integer type definitions and generic macros here, but avoid OS-specific and
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 4a3a105e1fdf..46f061a4458a 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -78,7 +78,7 @@ endif
CFLAGS_s390 = -m64
CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all))
-CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \
+CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -DNOLIBC_TEST \
$(call cc-option,-fno-stack-protector) \
$(CFLAGS_$(ARCH)) $(CFLAGS_STACKPROTECTOR)
Is this worth a new patch? I do think it is not required.
> > * Do we need to tune the order of the macros in unistd.h like this:
> >
> > #define _syscall(N, ...) __sysret(my_syscall##N(__VA_ARGS__))
> > #define _syscall_n(N, ...) _syscall(N, __VA_ARGS__)
> > #define __syscall_narg(_0, _1, _2, _3, _4, _5, _6, N, ...) N
> > #define _sycall_narg(...) __syscall_narg(__VA_ARGS__, 6, 5, 4, 3, 2, 1, 0)
> > #define syscall(...) _syscall_n(_sycall_narg(__VA_ARGS__), ##__VA_ARGS__)
> >
> > Before, It works but seems not put in using order:
> >
> > #define _syscall(N, ...) __sysret(my_syscall##N(__VA_ARGS__))
> > #define _sycall_narg(...) __syscall_narg(__VA_ARGS__, 6, 5, 4, 3, 2, 1, 0)
> > #define __syscall_narg(_0, _1, _2, _3, _4, _5, _6, N, ...) N
> > #define _syscall_n(N, ...) _syscall(N, __VA_ARGS__)
> > #define syscall(...) _syscall_n(_sycall_narg(__VA_ARGS__), ##__VA_ARGS__)
>
> Not sure it makes a big difference.
> If you want to change it, go for it.
>
Only switched two of them, oh, just found the '_sycall_narg' did miss a 's'
character, it may be really worth a patch now, I know why I focused on the
order so much, because the missing 's' made it not aligned well ;-)
> >
(...)
> > tools/include/nolibc/sys.h | 364 +++++-----------------------------
> > tools/include/nolibc/unistd.h | 11 +-
> > 2 files changed, 55 insertions(+), 320 deletions(-)
>
> For the full series:
>
> Reviewed-by: Thomas Weißschuh <[email protected]>
>
Thanks a lot, I'm really appreciated.
Best regards,
Zhangjin
> Thanks,
> Thomas