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

[Nageswara R Sastry]

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’.

@@ -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