linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value

Commit Message

Filip Bozuta July 2, 2020, 10:27 a.m. UTC

Function "print_fdset()" in "strace.c" is used to print the file descriptor
values in "print__newselect()" which prints arguments of syscall _newselect().
Until changes from this patch, this function was printing "," even after the
last value of the fd_set argument. This was changed in this patch by removing
this unnecessary "," after the last fd value and thus improving the estetics of
the _newselect() "-strace" print.

Implementation notes:

   The printing fix was made possible by using an existing function "get_comma()"
   which returns a "," or an empty string "" based on its argument (0 for "," and
   other for "").
---
 linux-user/strace.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

[e-mail] July 2, 2020, 11:18 a.m. UTC | #1

Patchew URL: https://patchew.org/QEMU/[e-mail]/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH] linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value
Type: series
Message-id: [e-mail]

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
Switched to a new branch 'test'
dff0674 linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value

=== OUTPUT BEGIN ===
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 21 lines checked

Commit dff0674696ce (linux-user: Fix "print_fdset()" in "strace.c" to not print ", " after last value) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to [e-mail]

Laurent Vivier July 2, 2020, 3:10 p.m. UTC | #2

Le 02/07/2020 à 12:27, Filip Bozuta a écrit :
> Function "print_fdset()" in "strace.c" is used to print the file descriptor
> values in "print__newselect()" which prints arguments of syscall _newselect().
> Until changes from this patch, this function was printing "," even after the
> last value of the fd_set argument. This was changed in this patch by removing
> this unnecessary "," after the last fd value and thus improving the estetics of
> the _newselect() "-strace" print.
> 
> Implementation notes:
> 
>    The printing fix was made possible by using an existing function "get_comma()"
>    which returns a "," or an empty string "" based on its argument (0 for "," and
>    other for "").
> ---
>  linux-user/strace.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 6044c66954..23ca5d88c8 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -541,6 +541,7 @@ static void
>  print_fdset(int n, abi_ulong target_fds_addr)
>  {
>      int i;
> +    int first = 1;
>  
>      qemu_log("[");
>      if( target_fds_addr ) {
> @@ -555,9 +556,12 @@ print_fdset(int n, abi_ulong target_fds_addr)
>              return;
>  
>          for (i=n; i>=0; i--) {
> -            if ((tswapal(target_fds[i / TARGET_ABI_BITS]) >> (i & (TARGET_ABI_BITS - 1))) & 1)
> -                qemu_log("%d,", i);
> +            if ((tswapal(target_fds[i / TARGET_ABI_BITS]) >>
> +                (i & (TARGET_ABI_BITS - 1))) & 1) {
> +                qemu_log("%s%d", get_comma(first), i);
> +                first = 0;
>              }
> +        }
>          unlock_user(target_fds, target_fds_addr, 0);
>      }
>      qemu_log("]");
> 

Please repost with your signed-off-by.

Reviewed-by: Laurent Vivier <[e-mail]>

Thanks,
Laurent

Patch

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 6044c66954..23ca5d88c8 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -541,6 +541,7 @@  static void
 print_fdset(int n, abi_ulong target_fds_addr)
 {
     int i;
+    int first = 1;
 
     qemu_log("[");
     if( target_fds_addr ) {
@@ -555,9 +556,12 @@  print_fdset(int n, abi_ulong target_fds_addr)
             return;
 
         for (i=n; i>=0; i--) {
-            if ((tswapal(target_fds[i / TARGET_ABI_BITS]) >> (i & (TARGET_ABI_BITS - 1))) & 1)
-                qemu_log("%d,", i);
+            if ((tswapal(target_fds[i / TARGET_ABI_BITS]) >>
+                (i & (TARGET_ABI_BITS - 1))) & 1) {
+                qemu_log("%s%d", get_comma(first), i);
+                first = 0;
             }
+        }
         unlock_user(target_fds, target_fds_addr, 0);
     }
     qemu_log("]");