Re: [PATCH v3 5/5] linux-user: Add strace support for printing arguments of some clock and time functions

[Laurent Vivier]

Le 22/07/2020 à 22:04, Filip Bozuta a écrit :
> This patch implements strace argument printing functionality for following 
> syscalls:
> 
>     * clock_getres, clock_gettime, clock_settime - clock and time functions
> 
>         int clock_getres(clockid_t clockid, struct timespec *res)
>         int clock_gettime(clockid_t clockid, struct timespec *tp)
>         int clock_settime(clockid_t clockid, const struct timespec *tp)
>         man page: https://man7.org/linux/man-pages/man2/clock_getres.2.html
> 
>     * gettimeofday - get time
> 
>         int gettimeofday(struct timeval *tv, struct timezone *tz)
>         man page: https://man7.org/linux/man-pages/man2/gettimeofday.2.html
> 
>     * getitimer, setitimer - get or set value of an interval timer
> 
>         int getitimer(int which, struct itimerval *curr_value)
>         int setitimer(int which, const struct itimerval *new_value,
>                       struct itimerval *old_value)
>         man page: https://man7.org/linux/man-pages/man2/getitimer.2.html
> 
> Implementation notes:
> 
>     All of the syscalls have some structue types as argument types and thus
>     a separate printing function was stated in file "strace.list" for each
>     of them. All of these functions use existing functions for their
>     appropriate structure types ("print_timeval()" and "print_timezone()").
> 
>     Functions "print_timespec()" and "print_itimerval()" were added in this
>     patch so that they can be used to print types "struct timespec" and
>     "struct itimerval" used by some of the syscalls. Function 
> "print_itimerval()"
>     uses the existing function "print_timeval()" to print fields of the
>     structure "struct itimerval" that are of type "struct timeval".
> 
>     Function "print_enums()", which was introduced in the previous patch, is 
> used
>     to print the interval timer type which is the first argument of 
> "getitimer()"
>     and "setitimer()". Also, this function is used to print the clock id which
>     is the first argument of "clock_getres()" and "clock_gettime()". For that
>     reason, the existing function "print_clockid()" was removed in this patch.
>     Existing function "print_clock_adjtime()" was also changed for this reason
>     to use "print_enums()".
> 
>     The existing function "print_timeval()" was changed a little so that it
>     prints the field names beside the values.
> 
>     Syscalls "clock_getres()" and "clock_gettime()" have the same number
>     and types of arguments and thus their print functions "print_clock_getres"
>     and "print_clock_gettime" share a common definition in file "strace.c".
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/strace.c    | 285 +++++++++++++++++++++++++++++++----------
>  linux-user/strace.list |  17 ++-
>  2 files changed, 230 insertions(+), 72 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index def92c4d73..aa5539f468 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
...
> @@ -1461,6 +1533,20 @@ print_timezone(abi_ulong tz_addr, int last)
>      }
>  }
>  
> +static void
> +print_itimerval(abi_ulong it_addr, int last)
> +{
> +    if (it_addr) {
> +        qemu_log("{it_interval=");
> +        print_timeval(it_addr, 0);
> +        qemu_log("it_value=");
> +        print_timeval(it_addr + sizeof(struct target_timeval), 1);
> +        qemu_log("}%s", get_comma(last));

You should use "target_timeval *it = lock_user(...);" and then
print_timeval(&it->it_interval, 0) and print_timeval(&i->it_value, 1)

> +    } else {
> +        qemu_log("NULL%s", get_comma(last));
> +    }
> +}
> +
>  #undef UNUSED
>  
>  #ifdef TARGET_NR_accept

Thanks,
Laurent