| Message ID | 1405091884-29955-3-git-send-email-Joakim.Tjernlund@transmode.se |
|---|---|
| State | New |
| Headers | show |
On 11 July 2014 16:18, Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote: > Used by AF_PACKET sockets > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > --- > 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
Peter Maydell <peter.maydell@linaro.org> wrote on 2014/07/11 18:00:25: > On 11 July 2014 16:18, Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote: > > Used by AF_PACKET sockets > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > > --- > > 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. Yes of course! I had messed with the code so much back and forth to make it work so I completely lost track. Jocke
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; + lladdr = (void *)addr; + + lladdr->sll_ifindex = tswap32(target_lladdr->sll_ifindex); + lladdr->sll_hatype = tswap16(target_lladdr->sll_hatype); + } unlock_user(target_saddr, target_addr, 0); return 0; diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index 8563027..329c9c5 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -121,6 +121,16 @@ struct target_sockaddr { uint8_t sa_data[14]; }; +struct target_sockaddr_ll { + uint16_t sll_family; /* Always AF_PACKET */ + uint16_t sll_protocol; /* Physical layer protocol */ + int sll_ifindex; /* Interface number */ + uint16_t sll_hatype; /* ARP hardware type */ + uint8_t sll_pkttype; /* Packet type */ + uint8_t sll_halen; /* Length of address */ + uint8_t sll_addr[8]; /* Physical layer address */ +}; + struct target_sock_filter { abi_ushort code; uint8_t jt;
Used by AF_PACKET sockets Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> --- linux-user/syscall.c | 8 ++++++++ linux-user/syscall_defs.h | 10 ++++++++++ 2 files changed, 18 insertions(+)