[libc-commits] [PATCH] D134919: [libc] add syscall function

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Can we call it `syscall_impl` instead of `internal_syscall` ?



================
Comment at: libc/include/llvm-libc-macros/linux/unistd-macros.h:22
+// ignored.
+#define syscall(...) __llvm_libc_syscall(__VA_ARGS__, 1, 2, 3, 4, 5, 6)
+
----------------
For better type checking, we should do something like this:

```
#define __syscall_helper(sysno, arg1, arg2, arg3, arg4, arg5, arg6, ...) \
    __llvm_libc_syscall((long)(sysno), (long)(arg1), (long)(arg2), (long)(arg3), \
                        (long)(arg4), (long)(arg5), (long)(arg6))
#define syscall(...) __syscall_helper(__VA_ARGS__, 1, 2, 3, 4, 5, 6, 7)
```


================
Comment at: libc/src/unistd/linux/syscall.cpp:19
+
+LLVM_LIBC_FUNCTION(long, __llvm_libc_syscall, (long number, ...)) {
+  va_list vlist;
----------------
This need not be a vararg function - it can take exactly 7 arguments, the syscall number plus 6 other args.


================
Comment at: libc/test/src/unistd/syscall_test.cpp:27
+
+  ASSERT_GE(__llvm_libc::syscall(SYS_gettid), 0l);
+  ASSERT_EQ(errno, 0);
----------------
This is very misleading as there is no member named `syscall` in the namespace `__llvm_libc`. So, can you add a comment below line 22 explaining how the macro substitution helps in actually picking the namespace qualified `__llvm_libc_syscall`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134919/new/

https://reviews.llvm.org/D134919