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

[Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>]

On 2018-02-07 19:27, Laurent Vivier wrote:
> Le 07/02/2018 à 10:49, no-reply@patchew.org 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:308: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:13: 
>> 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;
>                  break;

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;

Re-sending this mail because earlier one not reached the mailing list. 
Please accept my apologies if it is a duplicate.

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

-- 
Regards,
R.Nageswara Sastry