Re: [Qemu-devel] [PATCH 11/11] linux-user/syscall: simplify recvfrom()

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  linux-user/syscall.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b6b9beca5b..e082af2911 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4023,8 +4023,8 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, 
> size_t len, int flags,
>                              abi_ulong target_addr,
>                              abi_ulong target_addrlen)
>  {
> -    socklen_t addrlen;
> -    void *addr;
> +    socklen_t addrlen = 0;
> +    void *addr = NULL;
>      void *host_msg;
>      abi_long ret;
>  
> @@ -4046,12 +4046,9 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, 
> size_t len, int flags,
>          }
>  
>          addr = alloca(addrlen);
> -        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
> -                                      addr, &addrlen));
> -    } else {
> -        addr = NULL; /* To keep compiler quiet.  */
> -        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
>      }
> +    ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, addr, &addrlen));
> +
>      if (!is_error(ret)) {
>          if (fd_trans_host_to_target_data(fd)) {
>              ret = fd_trans_host_to_target_data(fd)(host_msg, ret);
> 

I think it would be better to pass a NULL pointer to the function for
addrlen if addr is NULL.

In man page we have:

    If the caller is not interested in the source address, src_addr and
    addrlen should be specified as NULL.

Something like:

...
-    socklen_t addrlen;
-    void *addr;
+    socklen_t *addrlen = NULL;
+    void *addr = NULL;
+    socklen_t target_len;
...
-        addr = alloca(addrlen);
-        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
-                                      addr, &addrlen));
-    } else {
-        addr = NULL; /* To keep compiler quiet.  */
-        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
+        addr = alloca(target_len);
+        addrlen = &target_len
     }
+    ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, addr,
addrlen));
...


Thanks,
Laurent