Re: [Qemu-devel] [PATCH 2/4] linux-user: impl. sockaddr_ll

[Peter Maydell]

On 11 July 2014 16:18, Joakim Tjernlund <address@hidden> wrote:
> Used by AF_PACKET sockets
>
> Signed-off-by: Joakim Tjernlund <address@hidden>
> ---
>  linux-user/syscall.c      |  8 ++++++++
>  linux-user/syscall_defs.h | 10 ++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1380f4e..a0e1ccc 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1140,6 +1140,14 @@ static inline abi_long target_to_host_sockaddr(struct 
> sockaddr *addr,
>
>      memcpy(addr, target_saddr, len);
>      addr->sa_family = sa_family;
> +    if (sa_family == AF_PACKET) {
> +           struct target_sockaddr_ll *target_lladdr, *lladdr;
> +           target_lladdr = (void *)target_saddr;

Casting to (struct target_sockaddr_ll *) would be nicer
than the void* cast.

> +           lladdr = (void *)addr;

This looks fishy -- shouldn't lladdr be  a struct sockaddr_ll, not
a struct target_sockaddr? Alternatively, if (as we seem to)
we rely on host and target layouts being the same, you might
as well use struct sockaddr_ll in both cases and add a comment
about our assumption.

> +
> +           lladdr->sll_ifindex = tswap32(target_lladdr->sll_ifindex);
> +           lladdr->sll_hatype = tswap16(target_lladdr->sll_hatype);

In fact we rely on memcpy() above to do most of the
data copying, so you might as well just say
   /* data already copied, just swap the fields which might
    * be different endianness in host and guest
    */
   lladdr->sll_ifindex = tswap32(lladdr->sll_ifindex);
   lladdr->sll_hatype = tswap16(lladdr->sll_hatype);

and drop the target_lladdr entirely.

thanks
-- PMM