Re: [Qemu-devel] [PATCH v4 10/16] linux-user: Split out some simple file syscalls

[Laurent Vivier]

Le 18/08/2018 à 21:01, Richard Henderson a écrit :
> This includes close, open, openat, read, readlink, readlinkat, write.
> 
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  linux-user/strace.c           |  64 -----
>  linux-user/syscall-file.inc.c | 440 ++++++++++++++++++++++++++++++++++
>  linux-user/syscall.c          | 393 ++----------------------------
>  linux-user/strace.list        |  21 --
>  4 files changed, 463 insertions(+), 455 deletions(-)
>  create mode 100644 linux-user/syscall-file.inc.c
> 
...
> +#include "syscall-file.inc.c"
> +
>  static const SyscallDef *syscall_table(int num)
>  {
> -#define S(NAME)  case TARGET_NR_##NAME: return &def_##NAME;
> +#define S(NAME)  case TARGET_NR_##NAME: return &def_##NAME
>  
>      switch (num) {
> +    /*
> +     * Unconditional syscalls.
> +     */
> +    S(close);
> +    S(openat);
> +    S(read);
> +    S(write);
> +
> +    /*
> +     * Conditional syscalls.
> +     */
> +#ifdef TARGET_NR_open
> +    S(open);
> +#endif
> +#ifdef TARGET_NR_readlink
> +    S(readlink);
> +#endif
> +#ifdef TARGET_NR_readlinkat
> +    S(readlinkat);
> +#endif
>      }
>      return NULL;

I don't understand why you need/want to duplicate the list of syscalls here.

If I modify your patch as following, it works without duplicating the list:

diff --git a/linux-user/syscall-file.def.c b/linux-user/syscall-file.def.c
new file mode 100644
index 0000000000..78b1bd0467
--- /dev/null
+++ b/linux-user/syscall-file.def.c
@@ -0,0 +1,13 @@
+SYSCALL_DEF(close, ARG_DEC);
+#ifdef TARGET_NR_open
+SYSCALL_DEF(open, ARG_STR, ARG_OPENFLAG, ARG_MODEFLAG);
+#endif
+SYSCALL_DEF(openat, ARG_ATDIRFD, ARG_STR, ARG_OPENFLAG, ARG_MODEFLAG);
+SYSCALL_DEF(read, ARG_DEC, ARG_PTR, ARG_DEC);
+#ifdef TARGET_NR_readlink
+SYSCALL_DEF(readlink, ARG_STR, ARG_PTR, ARG_DEC);
+#endif
+#ifdef TARGET_NR_readlinkat
+SYSCALL_DEF(readlinkat, ARG_ATDIRFD, ARG_STR, ARG_PTR, ARG_DEC);
+#endif
+SYSCALL_DEF(write, ARG_DEC, ARG_PTR, ARG_DEC);
diff --git a/linux-user/syscall-file.inc.c b/linux-user/syscall-file.inc.c
index aecc63682f..dda04200ed 100644
--- a/linux-user/syscall-file.inc.c
+++ b/linux-user/syscall-file.inc.c
@@ -307,21 +307,18 @@ SYSCALL_IMPL(close)
     fd_trans_unregister(arg1);
     return get_errno(close(arg1));
 }
-SYSCALL_DEF(close, ARG_DEC);

 #ifdef TARGET_NR_open
 SYSCALL_IMPL(open)
 {
     return do_openat(cpu_env, AT_FDCWD, arg1, arg2, arg3);
 }
-SYSCALL_DEF(open, ARG_STR, ARG_OPENFLAG, ARG_MODEFLAG);
 #endif

 SYSCALL_IMPL(openat)
 {
     return do_openat(cpu_env, arg1, arg2, arg3, arg4);
 }
-SYSCALL_DEF(openat, ARG_ATDIRFD, ARG_STR, ARG_OPENFLAG, ARG_MODEFLAG);

 SYSCALL_IMPL(read)
 {
@@ -346,7 +343,6 @@ SYSCALL_IMPL(read)
     unlock_user(p, arg2, ret);
     return ret;
 }
-SYSCALL_DEF(read, ARG_DEC, ARG_PTR, ARG_DEC);

 static abi_long do_readlink_proc_exe(char *buf, abi_ulong bufsiz)
 {
@@ -386,7 +382,6 @@ SYSCALL_IMPL(readlink)
     unlock_user(p, arg1, 0);
     return ret;
 }
-SYSCALL_DEF(readlink, ARG_STR, ARG_PTR, ARG_DEC);
 #endif

 #ifdef TARGET_NR_readlinkat
@@ -411,7 +406,6 @@ SYSCALL_IMPL(readlinkat)
     unlock_user(p, arg2, 0);
     return ret;
 }
-SYSCALL_DEF(readlinkat, ARG_ATDIRFD, ARG_STR, ARG_PTR, ARG_DEC);
 #endif

 SYSCALL_IMPL(write)
@@ -437,4 +431,4 @@ SYSCALL_IMPL(write)
     unlock_user(p, arg2, 0);
     return ret;
 }
-SYSCALL_DEF(write, ARG_DEC, ARG_PTR, ARG_DEC);
+#include "syscall-file.def.c"
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f757ae87b0..21e7700255 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12117,33 +12117,15 @@ static abi_long do_syscall1(void *cpu_env, int
num, abi_long arg1,

 static const SyscallDef *syscall_table(int num)
 {
-#define S(NAME)  case TARGET_NR_##NAME: return &def_##NAME
+#undef SYSCALL_DEF
+#define SYSCALL_DEF(NAME, ...)  case TARGET_NR_##NAME: return &def_##NAME

     switch (num) {
-    /*
-     * Unconditional syscalls.
-     */
-    S(close);
-    S(openat);
-    S(read);
-    S(write);
-
-    /*
-     * Conditional syscalls.
-     */
-#ifdef TARGET_NR_open
-    S(open);
-#endif
-#ifdef TARGET_NR_readlink
-    S(readlink);
-#endif
-#ifdef TARGET_NR_readlinkat
-    S(readlinkat);
-#endif
+#include "syscall-file.def.c"
     }
     return NULL;

-#undef S
+#undef SYSCALL_DEF
 }

 abi_long do_syscall(void *cpu_env, int num, abi_long arg1,