[SLOF] [PATCH] libnet: Fix compiler warnings with GCC 9

[Alexey Kardashevskiy]

On 20/09/2019 03:48, Thomas Huth wrote:
> When compiling the libnet code with GCC 9, there are some new
> compiler warnings popping up now:
> 
> ipv6.c: In function ‘handle_ipv6’:
> ipv6.c:152:21: warning: taking address of packed member of ‘struct ip6hdr’
>  may result in an unaligned pointer value [-Waddress-of-packed-member]
>   152 |  if (! find_ip6addr(&(ip6->dst)))
>       |                     ^~~~~~~~~~~
> ipv6.c: In function ‘ip6_checksum’:
> ipv6.c:455:2: warning: converting a packed ‘struct ip6hdr’ pointer
>  (alignment 1) to a ‘short unsigned int’ pointer (alignment 2) may result
>  in an unaligned pointer value [-Waddress-of-packed-member]
>   455 |  pip6h = (unsigned short *) &pseudo_ip6h;
>       |  ^~~~~
> In file included from ipv6.c:21:
> ipv6.h:86:8: note: defined here
>    86 | struct ip6hdr {
>       |        ^~~~~~
> ipv6.c:522:8: warning: converting a packed ‘struct icmp6hdr’ pointer
>  (alignment 1) to a ‘short unsigned int’ pointer (alignment 2) may result
>  in an unaligned pointer value [-Waddress-of-packed-member]
>   522 |        ip6h->pl >> 1);
>       |        ^~~~
> In file included from ipv6.c:22:
> icmpv6.h:123:8: note: defined here
>   123 | struct icmp6hdr {
>       |        ^~~~~~~~
> etc.
> 
> The entries in struct ip6hdr are naturally aligned, so we can simply
> drop the __attribute__ ((packed)) here and use a _Static_assert() for
> the correct size instead.
> icmp6hdr is a more complex struct since it contains a union of
> packed structs, but the entries before the union are naturally aligned,
> too, so we can silence the compiler warning by dropping the "packed"
> attribute from the struct icmp6hdr and just asserting that the union
> is at the correct offset.
> 
> Signed-off-by: Thomas Huth <thuth at redhat.com>
> ---
>  lib/libnet/icmpv6.h | 4 +++-
>  lib/libnet/ipv6.h   | 3 ++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libnet/icmpv6.h b/lib/libnet/icmpv6.h
> index 41b0c70..ba51524 100644
> --- a/lib/libnet/icmpv6.h
> +++ b/lib/libnet/icmpv6.h
> @@ -130,6 +130,8 @@ struct icmp6hdr {
>  		struct router_solicitation router_solicit;
>  		struct router_advertisement ra;
>  	} icmp6body;
> -} __attribute((packed));
> +};
> +_Static_assert((long)&(((struct icmp6hdr *)NULL)->icmp6body) == 4,
> +               "unexpected padding in icmp6hdr");
>  
>  #endif
> diff --git a/lib/libnet/ipv6.h b/lib/libnet/ipv6.h
> index 7b71b50..5fb718e 100644
> --- a/lib/libnet/ipv6.h
> +++ b/lib/libnet/ipv6.h
> @@ -90,7 +90,8 @@ struct ip6hdr {
>  	uint8_t  hl;		/**< Hop limit				*/
>  	ip6_addr_t src;		/**< IPv6 source address		*/
>  	ip6_addr_t dst;		/**< IPv6 destination address		*/
> -} __attribute((packed));


The (packed) rather explicitly says it is a binary format which is a
good thing.


> +};
> +_Static_assert(sizeof(struct ip6hdr) == 40, "unexpected padding in ip6hdr");


This hurts my eyes.

I propose this approach (gcc8 compile tested only, I still to compile my
own gcc9):

https://github.com/aik/SLOF/commit/2cdb340ac5026d0478eb873c0ad57bd6dc0a289f

Does this work? Basically instead of passing "unaligned" pointers, do
the right thing and pass values instead, we are not that much restricted
in SLOF to save bytes. Or disable this particular warning. What exactly
does it protect against? We can do unaligned memory access except few
very special cases such as spinlocks and this is not the case here.

Same comments apply to the USB patch. Thanks,



>  
>  /** \struct packeth
>   * Struct with pointers to headers within a packet
> 

-- 
Alexey