| Message ID | 20240807124306.52903-2-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: Trace sendto/recvfrom | expand |
On Wed, 2024-08-07 at 14:43 +0200, Philippe Mathieu-Daudé wrote: > When the %addr argument can not be accessed, a double comma > is logged (the final qemu_log call prepend a comma). Call > print_raw_param with last=1 to avoid the extra comma. > Remove spurious space. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > linux-user/strace.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/linux-user/strace.c b/linux-user/strace.c > index b4d1098170..73f81e66fc 100644 > --- a/linux-user/strace.c > +++ b/linux-user/strace.c > @@ -434,9 +434,9 @@ print_sockaddr(abi_ulong addr, abi_long addrlen, > int last) > } > unlock_user(sa, addr, 0); > } else { > - print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 0); > + print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 1); > } > - qemu_log(", "TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last)); > + qemu_log(","TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last)); > } > > static void I see why this works, but it feels a bit wrong semantically: addr is not the last argument. Wouldn't it be better to add commas to the preceding switch's cases? Anyhow: Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
On 10/2/24 00:54, Ilya Leoshkevich wrote: > On Wed, 2024-08-07 at 14:43 +0200, Philippe Mathieu-Daudé wrote: >> When the %addr argument can not be accessed, a double comma >> is logged (the final qemu_log call prepend a comma). Call >> print_raw_param with last=1 to avoid the extra comma. >> Remove spurious space. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> linux-user/strace.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/linux-user/strace.c b/linux-user/strace.c >> index b4d1098170..73f81e66fc 100644 >> --- a/linux-user/strace.c >> +++ b/linux-user/strace.c >> @@ -434,9 +434,9 @@ print_sockaddr(abi_ulong addr, abi_long addrlen, >> int last) >> } >> unlock_user(sa, addr, 0); >> } else { >> - print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 0); >> + print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 1); >> } >> - qemu_log(", "TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last)); >> + qemu_log(","TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last)); >> } >> >> static void > > I see why this works, but it feels a bit wrong semantically: addr is > not the last argument. > Wouldn't it be better to add commas to the preceding switch's cases? It would indeed. Adding the comma manually in the final log is odd. r~
diff --git a/linux-user/strace.c b/linux-user/strace.c index b4d1098170..73f81e66fc 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -434,9 +434,9 @@ print_sockaddr(abi_ulong addr, abi_long addrlen, int last) } unlock_user(sa, addr, 0); } else { - print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 0); + print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 1); } - qemu_log(", "TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last)); + qemu_log(","TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last)); } static void
When the %addr argument can not be accessed, a double comma is logged (the final qemu_log call prepend a comma). Call print_raw_param with last=1 to avoid the extra comma. Remove spurious space. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- linux-user/strace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)