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

2018-02-16 Thread Nageswara R Sastry 

On 2018-02-15 20:17, Laurent Vivier wrote:

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, no-re...@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’.


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


 1814 switch (cmsg->cmsg_level) {
 1815 case SOL_SOCKET:
 1816 switch (cmsg->cmsg_type) {
 1817 case SO_TIMESTAMP:
 1818 tgt_len = sizeof(struct target_timeval);
 1819 break;
 1820 default:
 1821 break;
 1822 }
 1823 default:
 1824 tgt_len = len;
 1825 break;
 1826 }


If setting tgt_len = len at 1820 then it get set to 'case SOL_SOCKET{ 
switch (cmsg_type's default) }' This place am not sure assingn tgt_len 
with len. To eliminate the gcc uninitialized error thought of 
initializing with '0'.



--
Regards,
R.Nageswara Sastry

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

2018-02-15 Thread 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, no-re...@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’.

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

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

2018-02-08 Thread Nageswara Sastry 
 On 07-Feb-2018, at 7:27 PM, Laurent Vivier 
 wrote:
 Le 07/02/2018 à 10:49, no-re...@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: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

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

2018-02-08 Thread Nageswara R Sastry 

On 2018-02-07 19:27, Laurent Vivier wrote:

Le 07/02/2018 à 10:49, no-re...@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

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

2018-02-07 Thread Nageswara R Sastry 
Detected by by Coverity (CID 1385425)
with out this break statement the assigned value of 'tgt_len' at
line 1824 will be replaced by value of 'tgt_len' at line 1830.

Signed-off-by: Nageswara R Sastry 
---
 linux-user/syscall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 74378947f0..3ecd533880 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1826,6 +1826,7 @@ static inline abi_long host_to_target_cmsg(struct 
target_msghdr *target_msgh,
 default:
 break;
 }
+break;
 default:
 tgt_len = len;
 break;
-- 
2.14.3 (Apple Git-98)

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

2018-02-07 Thread Laurent Vivier 
Le 07/02/2018 à 10:49, no-re...@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;
 }
+break;
 default:
 tgt_len = len;
 break;

Peter?

Thanks,
Laurent

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

2018-02-07 Thread Nageswara R Sastry 
Detected by by Coverity (CID 1385425)
with out this break statement the assigned value of 'tgt_len' at
line 1824 will be replaced by value of 'tgt_len' at line 1830.

Signed-off-by: Nageswara R Sastry 
---
 linux-user/syscall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 74378947f0..3ecd533880 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1826,6 +1826,7 @@ static inline abi_long host_to_target_cmsg(struct 
target_msghdr *target_msgh,
 default:
 break;
 }
+break;
 default:
 tgt_len = len;
 break;
-- 
2.14.3 (Apple Git-98)