[Qemu-devel] [PATCH] linux-user: Add strace support for statx.

Jim Wilson posted 1 patch 4 years, 11 months ago
Test s390x passed
Test checkpatch failed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190618235313.13223-1-jimw@sifive.com
Maintainers: Riku Voipio <riku.voipio@iki.fi>, Laurent Vivier <laurent@vivier.eu>
linux-user/strace.c    | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++
linux-user/strace.list |  3 ++
2 files changed, 89 insertions(+)
[Qemu-devel] [PATCH] linux-user: Add strace support for statx.
Posted by Jim Wilson 4 years, 11 months ago
All of the flags need to be conditional as old systems don't have statx
support.  Otherwise it works the same as other stat family syscalls.
This requires the pending patch to add statx support.

Tested on Ubuntu 16.04 (no host statx) and Ubuntu 19.04 (with host statx)
using a riscv32-linux toolchain.

Signed-off-by: Jim Wilson <jimw@sifive.com>
---
 linux-user/strace.c    | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list |  3 ++
 2 files changed, 89 insertions(+)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 6f72a74..c80e93b 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -976,6 +976,76 @@ UNUSED static struct flags msg_flags[] = {
     FLAG_END,
 };
 
+UNUSED static struct flags statx_flags[] = {
+#ifdef AT_EMPTY_PATH
+    FLAG_GENERIC(AT_EMPTY_PATH),
+#endif
+#ifdef AT_NO_AUTOMOUNT
+    FLAG_GENERIC(AT_NO_AUTOMOUNT),
+#endif
+#ifdef AT_SYMLINK_NOFOLLOW
+    FLAG_GENERIC(AT_SYMLINK_NOFOLLOW),
+#endif
+#ifdef AT_STATX_SYNC_AS_STAT
+    FLAG_GENERIC(AT_STATX_SYNC_AS_STAT),
+#endif
+#ifdef AT_STATX_FORCE_SYNC
+    FLAG_GENERIC(AT_STATX_FORCE_SYNC),
+#endif
+#ifdef AT_STATX_DONT_SYNC
+    FLAG_GENERIC(AT_STATX_DONT_SYNC),
+#endif
+    FLAG_END,
+};
+
+UNUSED static struct flags statx_mask[] = {
+/* This must come first, because it includes everything.  */
+#ifdef STATX_ALL
+    FLAG_GENERIC(STATX_ALL),
+#endif
+/* This must come second; it includes everything except STATX_BTIME.  */
+#ifdef STATX_BASIC_STATS
+    FLAG_GENERIC(STATX_BASIC_STATS),
+#endif
+#ifdef STATX_TYPE
+    FLAG_GENERIC(STATX_TYPE),
+#endif
+#ifdef STATX_MODE
+    FLAG_GENERIC(STATX_MODE),
+#endif
+#ifdef STATX_NLINK
+    FLAG_GENERIC(STATX_NLINK),
+#endif
+#ifdef STATX_UID
+    FLAG_GENERIC(STATX_UID),
+#endif
+#ifdef STATX_GID
+    FLAG_GENERIC(STATX_GID),
+#endif
+#ifdef STATX_ATIME
+    FLAG_GENERIC(STATX_ATIME),
+#endif
+#ifdef STATX_MTIME
+    FLAG_GENERIC(STATX_MTIME),
+#endif
+#ifdef STATX_CTIME
+    FLAG_GENERIC(STATX_CTIME),
+#endif
+#ifdef STATX_INO
+    FLAG_GENERIC(STATX_INO),
+#endif
+#ifdef STATX_SIZE
+    FLAG_GENERIC(STATX_SIZE),
+#endif
+#ifdef STATX_BLOCKS
+    FLAG_GENERIC(STATX_BLOCKS),
+#endif
+#ifdef STATX_BTIME
+    FLAG_GENERIC(STATX_BTIME),
+#endif
+    FLAG_END,
+};
+
 /*
  * print_xxx utility functions.  These are used to print syscall
  * parameters in certain format.  All of these have parameter
@@ -2611,6 +2681,22 @@ print_tgkill(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_statx
+static void
+print_statx(const struct syscallname *name,
+            abi_long arg0, abi_long arg1, abi_long arg2,
+            abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_at_dirfd(arg0, 0);
+    print_string(arg1, 0);
+    print_flags(statx_flags, arg2, 0);
+    print_flags(statx_mask, arg3, 0);
+    print_pointer(arg4, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 /*
  * An array of all of the syscalls we know about
  */
diff --git a/linux-user/strace.list b/linux-user/strace.list
index db21ce4..63a9466 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1650,3 +1650,6 @@
 #ifdef TARGET_NR_atomic_barrier
 { TARGET_NR_atomic_barrier, "atomic_barrier", NULL, NULL, NULL },
 #endif
+#ifdef TARGET_NR_statx
+{ TARGET_NR_statx, "statx", NULL, print_statx, NULL },
+#endif
-- 
2.7.4


Re: [Qemu-devel] [PATCH] linux-user: Add strace support for statx.
Posted by no-reply@patchew.org 4 years, 11 months ago
Patchew URL: https://patchew.org/QEMU/20190618235313.13223-1-jimw@sifive.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH] linux-user: Add strace support for statx.
Type: series
Message-id: 20190618235313.13223-1-jimw@sifive.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190618235313.13223-1-jimw@sifive.com -> patchew/20190618235313.13223-1-jimw@sifive.com
Switched to a new branch 'test'
dde446f95f linux-user: Add strace support for statx.

=== OUTPUT BEGIN ===
ERROR: storage class should be at the beginning of the declaration
#25: FILE: linux-user/strace.c:979:
+UNUSED static struct flags statx_flags[] = {

ERROR: storage class should be at the beginning of the declaration
#47: FILE: linux-user/strace.c:1001:
+UNUSED static struct flags statx_mask[] = {

total: 2 errors, 0 warnings, 104 lines checked

Commit dde446f95fb2 (linux-user: Add strace support for statx.) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190618235313.13223-1-jimw@sifive.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH] linux-user: Add strace support for statx.
Posted by Jim Wilson 4 years, 11 months ago
On Tue, Jun 18, 2019 at 5:09 PM <no-reply@patchew.org> wrote:
> === OUTPUT BEGIN ===
> ERROR: storage class should be at the beginning of the declaration
> #25: FILE: linux-user/strace.c:979:
> +UNUSED static struct flags statx_flags[] = {

It is complaining about UNUSED, which is a macro that expands to
attribute unused, but this is the exact same C syntax used in the rest
of the file.  I think this is a bug in the checkpatch script as this
looks like it should be allowed.  Or maybe there is an exception for
this one file?

Jim

Re: [Qemu-devel] [PATCH] linux-user: Add strace support for statx.
Posted by Laurent Vivier 4 years, 11 months ago
Le 19/06/2019 à 01:53, Jim Wilson a écrit :
> All of the flags need to be conditional as old systems don't have statx
> support.  Otherwise it works the same as other stat family syscalls.
> This requires the pending patch to add statx support.
> 
> Tested on Ubuntu 16.04 (no host statx) and Ubuntu 19.04 (with host statx)
> using a riscv32-linux toolchain.
> 
> Signed-off-by: Jim Wilson <jimw@sifive.com>
> ---
>  linux-user/strace.c    | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  3 ++
>  2 files changed, 89 insertions(+)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 6f72a74..c80e93b 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -976,6 +976,76 @@ UNUSED static struct flags msg_flags[] = {
>      FLAG_END,
>  };
>  
> +UNUSED static struct flags statx_flags[] = {
> +#ifdef AT_EMPTY_PATH
> +    FLAG_GENERIC(AT_EMPTY_PATH),
> +#endif
> +#ifdef AT_NO_AUTOMOUNT
> +    FLAG_GENERIC(AT_NO_AUTOMOUNT),
> +#endif
> +#ifdef AT_SYMLINK_NOFOLLOW
> +    FLAG_GENERIC(AT_SYMLINK_NOFOLLOW),
> +#endif
> +#ifdef AT_STATX_SYNC_AS_STAT
> +    FLAG_GENERIC(AT_STATX_SYNC_AS_STAT),
> +#endif
> +#ifdef AT_STATX_FORCE_SYNC
> +    FLAG_GENERIC(AT_STATX_FORCE_SYNC),
> +#endif
> +#ifdef AT_STATX_DONT_SYNC
> +    FLAG_GENERIC(AT_STATX_DONT_SYNC),
> +#endif
> +    FLAG_END,
> +};
> +
> +UNUSED static struct flags statx_mask[] = {
> +/* This must come first, because it includes everything.  */
> +#ifdef STATX_ALL
> +    FLAG_GENERIC(STATX_ALL),
> +#endif
> +/* This must come second; it includes everything except STATX_BTIME.  */
> +#ifdef STATX_BASIC_STATS
> +    FLAG_GENERIC(STATX_BASIC_STATS),
> +#endif
> +#ifdef STATX_TYPE
> +    FLAG_GENERIC(STATX_TYPE),
> +#endif
> +#ifdef STATX_MODE
> +    FLAG_GENERIC(STATX_MODE),
> +#endif
> +#ifdef STATX_NLINK
> +    FLAG_GENERIC(STATX_NLINK),
> +#endif
> +#ifdef STATX_UID
> +    FLAG_GENERIC(STATX_UID),
> +#endif
> +#ifdef STATX_GID
> +    FLAG_GENERIC(STATX_GID),
> +#endif
> +#ifdef STATX_ATIME
> +    FLAG_GENERIC(STATX_ATIME),
> +#endif
> +#ifdef STATX_MTIME
> +    FLAG_GENERIC(STATX_MTIME),
> +#endif
> +#ifdef STATX_CTIME
> +    FLAG_GENERIC(STATX_CTIME),
> +#endif
> +#ifdef STATX_INO
> +    FLAG_GENERIC(STATX_INO),
> +#endif
> +#ifdef STATX_SIZE
> +    FLAG_GENERIC(STATX_SIZE),
> +#endif
> +#ifdef STATX_BLOCKS
> +    FLAG_GENERIC(STATX_BLOCKS),
> +#endif
> +#ifdef STATX_BTIME
> +    FLAG_GENERIC(STATX_BTIME),
> +#endif
> +    FLAG_END,
> +};
> +
>  /*
>   * print_xxx utility functions.  These are used to print syscall
>   * parameters in certain format.  All of these have parameter
> @@ -2611,6 +2681,22 @@ print_tgkill(const struct syscallname *name,
>  }
>  #endif
>  
> +#ifdef TARGET_NR_statx
> +static void
> +print_statx(const struct syscallname *name,
> +            abi_long arg0, abi_long arg1, abi_long arg2,
> +            abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_at_dirfd(arg0, 0);
> +    print_string(arg1, 0);
> +    print_flags(statx_flags, arg2, 0);
> +    print_flags(statx_mask, arg3, 0);
> +    print_pointer(arg4, 1);
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
>  /*
>   * An array of all of the syscalls we know about
>   */
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index db21ce4..63a9466 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1650,3 +1650,6 @@
>  #ifdef TARGET_NR_atomic_barrier
>  { TARGET_NR_atomic_barrier, "atomic_barrier", NULL, NULL, NULL },
>  #endif
> +#ifdef TARGET_NR_statx
> +{ TARGET_NR_statx, "statx", NULL, print_statx, NULL },
> +#endif
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>