Re: [Qemu-devel] [patch] linux-user/syscall.c: Fix missing break for host_to_target_cmsg

[Nageswara Sastry]

     On 07-Feb-2018, at 7:27 PM, Laurent Vivier <address@hidden>
     wrote:
     Le 07/02/2018 à 10:49, address@hidden a écrit :

     Hi,
     This series failed build test on s390x host. Please find the details
     below.

     ...

     CC      aarch64_be-linux-user/linux-user/syscall.o
     In file included from
     /var/tmp/patchew-tester-tmp-ewjgn083/src/linux-user/qemu.h:16:0,
                    from
     /var/tmp/patchew-tester-tmp-ewjgn083/src/linux-user/syscall.c:118:
     /var/tmp/patchew-tester-tmp-ewjgn083/src/linux-user/syscall.c: In
     function ‘do_sendrecvmsg_locked’:
     /var/tmp/patchew-tester-tmp-ewjgn083/src/linux-user/syscall_defs.h:3
     08:61: error: ‘tgt_len’ may be used uninitialized in this
     function [-Werror=maybe-uninitialized]
     #define TARGET_CMSG_LEN(len) (sizeof(struct target_cmsghdr) + (len))
                                                                ^
     /var/tmp/patchew-tester-tmp-ewjgn083/src/linux-user/syscall.c:1797:1
     3: note: ‘tgt_len’ was declared here
            int tgt_len, tgt_space;
                ^~~~~~~

     it seems gcc disagrees with Coverity...
     I think this should fixed like:
     diff --git a/linux-user/syscall.c b/linux-user/syscall.c
     index 74378947f0..d7fbe334eb 100644
     --- a/linux-user/syscall.c
     +++ b/linux-user/syscall.c
     @@ -1824,8 +1824,10 @@ static inline abi_long
     host_to_target_cmsg(struct
     target_msghdr *target_msgh,
                    tgt_len = sizeof(struct target_timeval);
                    break;
                default:
     +                tgt_len = len;

   In my view this will result in assigning a wrong value to ‘tgt_len’
   at this ‘switch-case’ condition.
   Instead looking at the option of initializing ‘tgt_len' to ‘0’.
   @@ -1789,7 +1789,7 @@
           void *target_data = TARGET_CMSG_DATA(target_cmsg);
           int len = cmsg->cmsg_len - sizeof(struct cmsghdr);
   -        int tgt_len, tgt_space;
   +        int tgt_len = 0, tgt_space;
           /* We never copy a half-header but may copy half-data;
            * this is Linux's behaviour in put_cmsg(). Note that
   @@ -1821,6 +1821,7 @@
               default:
                   break;
               }
   +        break;
           default:
               tgt_len = len;
               break;

                    break;
                }
     +            break;
            default:
                tgt_len = len;
                break;
     Peter?
     Thanks,
     Laurent