Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()

[Helge Deller]

On 12/23/22 12:01, Philippe Mathieu-Daudé wrote:
> On 23/12/22 11:53, Helge Deller wrote:
> > On 12/23/22 11:50, Philippe Mathieu-Daudé wrote:
> > > On 23/12/22 11:01, Helge Deller wrote:
> > > > Make the strace look nicer for those two syscalls.
> > > >
> > > > Signed-off-by: Helge Deller <deller@gmx.de>
> > > > ---
> > > >   linux-user/strace.list | 4 ++--
> > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/linux-user/strace.list b/linux-user/strace.list
> > > > index f9254725a1..909298099e 100644
> > > > --- a/linux-user/strace.list
> > > > +++ b/linux-user/strace.list
> > > > @@ -1043,7 +1043,7 @@
> > > >   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
> > > >   #endif
> > > >   #ifdef TARGET_NR_personality
> > > > -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
> > > > +{ TARGET_NR_personality, "personality" , "%s(%p)", NULL, 
> > > > print_syscall_ret_addr },
> > >
> > > Shouldn't this be:
> > >
> > >     { TARGET_NR_personality, "personality" , "%s(%u)", NULL, NULL },
> >
> > Basically yes, but...
> > it's a bitmap, so printing it as hex value (similiar to a pointer)
> > is easier to read/analyze.
>
> Oh, good point. Then "%s(0x"TARGET_ABI_FMT_lx")" is self-explicit.

Hmm ... I don't see that as any benefit for the user and the output is the same.

> Also for clarity:
>
> #define print_syscall_ret_persona print_syscall_ret_addr
>
> So what do you think of:
>
> { TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")",
>     NULL, print_syscall_ret_persona },

This change does make sense if someone fully implements the
strace of personality() including showing the flags as string.
Until then it's IMHO just one more function which uses space
and gains nothing.

Helge