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

[Laurent Vivier]

Le 08/02/2018 à 10:56, Nageswara R Sastry a écrit :
> On 2018-02-07 19:27, Laurent Vivier 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: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’.

According to the comment above the switch():

        /* Payload types which need a different size of payload on
         * the target must adjust tgt_len here.
         */

So "tgt_len" must be "len" by default, except if it needs to be adjusted
(currently only for SO_TIMESTAMP), so I don't understand why it should
be set to "0".

Thanks,
Laurent