[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [patch] linux-user/syscall.c: Fix missing break for hos
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [patch] linux-user/syscall.c: Fix missing break for host_to_target_cmsg |
Date: |
Thu, 15 Feb 2018 15:47:44 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
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