Re: [Qemu-devel] [PATCH 04/12] slirp: add in6_multicast() and use it instead of IN6_IS_ADDR_MULTICAST()

[Thomas Huth]

On 08.01.2018 18:28, Philippe Mathieu-Daudé wrote:
> Host: Mac OS 10.12.5
> Compiler: Apple LLVM version 8.1.0 (clang-802.0.42)
> 
>   slirp/ip6_icmp.c:79:32: warning: taking address of packed member 'ip_src' 
> of class or
>         structure 'ip6' may result in an unaligned pointer value
>         [-Waddress-of-packed-member]
>       if (IN6_IS_ADDR_MULTICAST(&ip->ip_src) ||
>                                  ^~~~~~~~~~
>   /usr/include/netinet6/in6.h:299:36: note: expanded from macro 
> 'IN6_IS_ADDR_MULTICAST'
>   #define IN6_IS_ADDR_MULTICAST(a)        ((a)->s6_addr[0] == 0xff)
>                                             ^
> 
> Reported-by: John Arbuckle <address@hidden>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  slirp/ip6.h       |  5 +++++
>  slirp/ip6_icmp.c  | 10 +++++-----
>  slirp/ndp_table.c |  4 ++--
>  3 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/slirp/ip6.h b/slirp/ip6.h
> index b1bea43b3c..6c5d4eeaa3 100644
> --- a/slirp/ip6.h
> +++ b/slirp/ip6.h
> @@ -93,6 +93,11 @@ static inline bool in6_equal_mach(const struct in6_addr *a,
>  #define in6_zero(a)\
>      (in6_equal(a, &(struct in6_addr)ZERO_ADDR))

I think you should put a comment here to say why we need our own
function and can not use the IN6_IS_ADDR_MULTICAST macro instead -
otherwise people might be confused when looking at this code in a year
or two. (and now I've also understood why you're poisining the macros in
the next patch ... a comment in the code there would certainly not hurt
either).

> +static inline bool in6_multicast(const struct in6_addr *a)
> +{
> +    return a->s6_addr[0] == 0xff;
> +}
> +
>  /* Compute emulated host MAC address from its ipv6 address */
>  static inline void in6_compute_ethaddr(struct in6_addr ip,
>                                         uint8_t eth[ETH_ALEN])

 Thomas