Re: [PATCH 3/5] linux-user: Add strace support for printing arguments of lseek()

[Laurent Vivier]

Le 02/06/2020 à 13:53, Filip Bozuta a écrit :
> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
> 
> This patch implements strace argument printing functionality for syscall:
> 
>     *lseek - reposition read/write file offset
> 
>          off_t lseek(int fd, off_t offset, int whence)
>          man page: https://www.man7.org/linux/man-pages/man2/lseek.2.html
> 
> Implementation notes:
> 
>     The syscall's third argument "whence" has predefined values:
>     "SEEK_SET","SEEK_CUR","SEEK_END","SEEK_DATA","SEEK_HOLE"
>     and thus a separate printing function "print_lseek" was stated
>     in file "strace.list". This function is defined in "strace.c"
>     by using an existing function "print_raw_param()" to print
>     the first and second argument and a switch(case) statement
>     for the predefined values of the third argument.
>     Values "SEEK_DATA" and "SEEK_HOLE" are defined in kernel version 3.1.
>     That is the reason why case statements for these values are
>     enwrapped in #ifdef directive.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/strace.c    | 32 ++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 5cf419989c..b7f012f1b5 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1768,6 +1768,38 @@ print__llseek(const struct syscallname *name,
>  }
>  #endif
>  
> +#ifdef TARGET_NR_lseek
> +static void
> +print_lseek(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_raw_param("%d", arg0, 0);
> +    print_raw_param("%ld", arg1, 0);

TARGET_ABI_FMT_ld is better than "%ld", because abi_long can be a 32bit
or a 64bit value, and be different between host and target.

> +    switch (arg2) {
> +    case SEEK_SET:
> +        qemu_log("SEEK_SET"); break;
> +    case SEEK_CUR:
> +        qemu_log("SEEK_CUR"); break;
> +    case SEEK_END:
> +        qemu_log("SEEK_END"); break;
> +#ifdef SEEK_DATA
> +    case SEEK_DATA:
> +        qemu_log("SEEK_DATA"); break;
> +#endif
> +#ifdef SEEK_HOLE
> +    case SEEK_HOLE:
> +        qemu_log("SEEK_HOLE"); break;
> +#endif
> +    default:
> +        print_raw_param("%#x", arg2, 1);
> +        qemu_log(" /* SEEK_??? */");

remove this line ^^^

> +    }
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
>  #if defined(TARGET_NR_socket)
>  static void
>  print_socket(const struct syscallname *name,
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index 8d51c54bca..5a56212532 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -513,7 +513,7 @@
>  { TARGET_NR_lremovexattr, "lremovexattr" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_lseek
> -{ TARGET_NR_lseek, "lseek" , NULL, NULL, NULL },
> +{ TARGET_NR_lseek, "lseek" , NULL, print_lseek, NULL },
>  #endif
>  #ifdef TARGET_NR_lsetxattr
>  { TARGET_NR_lsetxattr, "lsetxattr" , NULL, NULL, NULL },
> 

Thanks,
Laurent