Re: [PATCH v2] linux-user: Improve strace output of pread64() and pwrite64()

[Laurent Vivier]

Le 30/01/2023 à 23:11, Helge Deller a écrit :
> On 1/30/23 10:26, Laurent Vivier wrote:
> > Le 27/01/2023 à 21:58, Helge Deller a écrit :
> > > Make the strace look nicer for those two syscalls.
> > >
> > > Signed-off-by: Helge Deller <deller@gmx.de>
> > > ---
> > > v2: Use regpairs_aligned() and target_offset64(), noticed by Laurent Vivier
> > >
> > > diff --git a/linux-user/strace.c b/linux-user/strace.c
> > > index 82dc1a1e20..379536f5c9 100644
> > > --- a/linux-user/strace.c
> > > +++ b/linux-user/strace.c
> > > @@ -3824,6 +3824,25 @@ print_rlimit64(abi_ulong rlim_addr, int last)
> > >       }
> > >   }
> > >
> > > +#if defined(TARGET_NR_pread64) || defined(TARGET_NR_pwrite64)
> > > +static void
> > > +print_preadwrite64(CPUArchState *cpu_env, const struct syscallname *name,
> > > +           abi_long arg0, abi_long arg1, abi_long arg2,
> > > +           abi_long arg3, abi_long arg4, abi_long arg5)
> > > +{
> > > +    if (regpairs_aligned(cpu_env, TARGET_NR_pread64)) {
> > > +        arg3 = arg4;
> > > +        arg4 = arg5;
> > > +    }
> > > +    print_syscall_prologue(name);
> > > +    print_raw_param("%d", arg0, 0);
> > > +    print_pointer(arg1, 0);
> > > +    print_raw_param("%d", arg2, 0);
> > > +    qemu_log("%lld", (long long)target_offset64(arg3, arg4));
> >
> > better to use:
> >
> > print_raw_param("%" PRIu64, target_offset64(arg3, arg4), 1);
>
> I thought of that as well, but that won't work, as print_raw_param()
> takes an "abi_long" value, which is just a 32-bit value on 32-bit targets.
> See print_rlimit64(), it's used there with qemu_log() as well.

Yes, you're right.

But even with qemu_log() I would prefer you use "%"PRIu64 rather than %lld.

Or better define a print_raw_param64() (or similar) and update print_fallocate(), print_truncate64() 
and print_ftruncate64().

Thanks,
Laurent