All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] linux-user: Improve strace output of personality() and sysinfo()
@ 2022-12-23 10:01 Helge Deller
  2022-12-23 10:50 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 10+ messages in thread
From: Helge Deller @ 2022-12-23 10:01 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel; +Cc: Helge Deller

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 },
 #endif
 #ifdef TARGET_NR_pipe
 { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
@@ -1502,7 +1502,7 @@
 { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_sysinfo
-{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
+{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_sys_kexec_load
 { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
--
2.38.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
  2022-12-23 10:01 [PATCH] linux-user: Improve strace output of personality() and sysinfo() Helge Deller
@ 2022-12-23 10:50 ` Philippe Mathieu-Daudé
  2022-12-23 10:53   ` Helge Deller
  0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-23 10:50 UTC (permalink / raw)
  To: Helge Deller, Laurent Vivier, qemu-devel

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 },

?

>   #endif
>   #ifdef TARGET_NR_pipe
>   { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
> @@ -1502,7 +1502,7 @@
>   { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sysinfo
> -{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
> +{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sys_kexec_load
>   { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
> --
> 2.38.1
> 
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
  2022-12-23 10:50 ` Philippe Mathieu-Daudé
@ 2022-12-23 10:53   ` Helge Deller
  2022-12-23 11:01     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 10+ messages in thread
From: Helge Deller @ 2022-12-23 10:53 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Laurent Vivier, qemu-devel

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.

Helge

>
> ?
>
>>   #endif
>>   #ifdef TARGET_NR_pipe
>>   { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
>> @@ -1502,7 +1502,7 @@
>>   { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
>>   #endif
>>   #ifdef TARGET_NR_sysinfo
>> -{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
>> +{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
>>   #endif
>>   #ifdef TARGET_NR_sys_kexec_load
>>   { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
>> --
>> 2.38.1
>>
>>
>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
  2022-12-23 10:53   ` Helge Deller
@ 2022-12-23 11:01     ` Philippe Mathieu-Daudé
  2022-12-23 12:27       ` Helge Deller
  0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-23 11:01 UTC (permalink / raw)
  To: Helge Deller, Laurent Vivier, qemu-devel

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.

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 },

?


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
  2022-12-23 11:01     ` Philippe Mathieu-Daudé
@ 2022-12-23 12:27       ` Helge Deller
  2023-01-26 16:25         ` Laurent Vivier
  0 siblings, 1 reply; 10+ messages in thread
From: Helge Deller @ 2022-12-23 12:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Laurent Vivier, qemu-devel

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] linux-user: Improve strace output of personality() and sysinfo()
  2022-12-23 12:27       ` Helge Deller
@ 2023-01-26 16:25         ` Laurent Vivier
  2023-01-27 20:18           ` [PATCH v2] " Helge Deller
  0 siblings, 1 reply; 10+ messages in thread
From: Laurent Vivier @ 2023-01-26 16:25 UTC (permalink / raw)
  To: Helge Deller, Philippe Mathieu-Daudé, qemu-devel

Le 23/12/2022 à 13:27, Helge Deller a écrit :
> 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.
> 

I agree with Philippe for this part, it's not a pointer, don't use %p.

Thanks,
Laurent





^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] linux-user: Improve strace output of personality() and sysinfo()
  2023-01-26 16:25         ` Laurent Vivier
@ 2023-01-27 20:18           ` Helge Deller
  2023-01-27 22:53             ` Richard Henderson
                               ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Helge Deller @ 2023-01-27 20:18 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Helge Deller, Philippe Mathieu-Daudé, qemu-devel

Make the strace look nicer for those two syscalls.

Signed-off-by: Helge Deller <deller@gmx.de>

--
v2: use TARGET_ABI_FMT_lx instead of %p in personality output
    as suggested by Philippe Mathieu-Daudé and Laurent Vivier


diff --git a/linux-user/strace.list b/linux-user/strace.list
index f9254725a1..703c0f1608 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1043,7 +1043,8 @@
 { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_personality
-{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
+{ TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")", NULL,
+  print_syscall_ret_addr },
 #endif
 #ifdef TARGET_NR_pipe
 { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
@@ -1502,7 +1503,7 @@
 { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_sysinfo
-{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
+{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_sys_kexec_load
 { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] linux-user: Improve strace output of personality() and sysinfo()
  2023-01-27 20:18           ` [PATCH v2] " Helge Deller
@ 2023-01-27 22:53             ` Richard Henderson
  2023-01-30  8:44             ` Laurent Vivier
  2023-01-30  8:52             ` Laurent Vivier
  2 siblings, 0 replies; 10+ messages in thread
From: Richard Henderson @ 2023-01-27 22:53 UTC (permalink / raw)
  To: Helge Deller, Laurent Vivier; +Cc: Philippe Mathieu-Daudé, qemu-devel

On 1/27/23 10:18, Helge Deller wrote:
> Make the strace look nicer for those two syscalls.
> 
> Signed-off-by: Helge Deller<deller@gmx.de>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] linux-user: Improve strace output of personality() and sysinfo()
  2023-01-27 20:18           ` [PATCH v2] " Helge Deller
  2023-01-27 22:53             ` Richard Henderson
@ 2023-01-30  8:44             ` Laurent Vivier
  2023-01-30  8:52             ` Laurent Vivier
  2 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2023-01-30  8:44 UTC (permalink / raw)
  To: Helge Deller; +Cc: Philippe Mathieu-Daudé, qemu-devel

Le 27/01/2023 à 21:18, Helge Deller a écrit :
> Make the strace look nicer for those two syscalls.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> --
> v2: use TARGET_ABI_FMT_lx instead of %p in personality output
>      as suggested by Philippe Mathieu-Daudé and Laurent Vivier
> 
> 
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index f9254725a1..703c0f1608 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1043,7 +1043,8 @@
>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_personality
> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
> +{ TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")", NULL,
> +  print_syscall_ret_addr },
>   #endif
>   #ifdef TARGET_NR_pipe
>   { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
> @@ -1502,7 +1503,7 @@
>   { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sysinfo
> -{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
> +{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sys_kexec_load
>   { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
> 

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] linux-user: Improve strace output of personality() and sysinfo()
  2023-01-27 20:18           ` [PATCH v2] " Helge Deller
  2023-01-27 22:53             ` Richard Henderson
  2023-01-30  8:44             ` Laurent Vivier
@ 2023-01-30  8:52             ` Laurent Vivier
  2 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2023-01-30  8:52 UTC (permalink / raw)
  To: Helge Deller; +Cc: Philippe Mathieu-Daudé, qemu-devel

Le 27/01/2023 à 21:18, Helge Deller a écrit :
> Make the strace look nicer for those two syscalls.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> --
> v2: use TARGET_ABI_FMT_lx instead of %p in personality output
>      as suggested by Philippe Mathieu-Daudé and Laurent Vivier
> 
> 
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index f9254725a1..703c0f1608 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -1043,7 +1043,8 @@
>   { TARGET_NR_perfctr, "perfctr" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_personality
> -{ TARGET_NR_personality, "personality" , NULL, NULL, NULL },
> +{ TARGET_NR_personality, "personality" , "%s(0x"TARGET_ABI_FMT_lx")", NULL,
> +  print_syscall_ret_addr },
>   #endif
>   #ifdef TARGET_NR_pipe
>   { TARGET_NR_pipe, "pipe" , NULL, NULL, NULL },
> @@ -1502,7 +1503,7 @@
>   { TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sysinfo
> -{ TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL },
> +{ TARGET_NR_sysinfo, "sysinfo" , "%s(%p)", NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_sys_kexec_load
>   { TARGET_NR_sys_kexec_load, "sys_kexec_load" , NULL, NULL, NULL },
> 

Applied to my linux-user-for-8.0 branch.

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-01-30  8:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-23 10:01 [PATCH] linux-user: Improve strace output of personality() and sysinfo() Helge Deller
2022-12-23 10:50 ` Philippe Mathieu-Daudé
2022-12-23 10:53   ` Helge Deller
2022-12-23 11:01     ` Philippe Mathieu-Daudé
2022-12-23 12:27       ` Helge Deller
2023-01-26 16:25         ` Laurent Vivier
2023-01-27 20:18           ` [PATCH v2] " Helge Deller
2023-01-27 22:53             ` Richard Henderson
2023-01-30  8:44             ` Laurent Vivier
2023-01-30  8:52             ` Laurent Vivier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.