Re: [RFC] iproute2: ipila warning

[Petr Machata <petrm@nvidia.com>]

Stephen Hemminger <stephen@networkplumber.org> writes:

> Building current code with Debian stable Gcc 12.2.0 see this warning.
>
>     CC       ipila.o
> ipila.c: In function ‘print_ila_locid’:
> ipila.c:57:32: warning: ‘addr’ may be used uninitialized [-Wmaybe-uninitialized]
>    57 |                 v = ntohs(words[i]);
>       |                                ^
> ipila.c:69:13: note: ‘addr’ declared here
>    69 | static void print_ila_locid(const char *tag, int attr, struct rtattr *tb[])
>       |             ^~~~~~~~~~~~~~~
>
> Looks like a Gcc aliasing bug.
> Relevant snippets.
>
> static void print_addr64(__u64 addr, char *buff, size_t len)
> {
> 	__u16 *words = (__u16 *)&addr;
> 	__u16 v;
> 	int i, ret;
> 	size_t written = 0;
> 	char *sep = ":";
>
> 	for (i = 0; i < 4; i++) {
> 		v = ntohs(words[i]);
> ...
>
>
> static void print_ila_locid(const char *tag, int attr, struct rtattr *tb[])
> {
> 	char abuf[256];
>
> 	if (tb[attr])
> 		print_addr64(rta_getattr_u64(tb[attr]),
> 			     abuf, sizeof(abuf));
>
> One solution would be to use a union.
> Other would be to use some variation of no-strict aliasing.
>
> --- a/ip/ipila.c
> +++ b/ip/ipila.c
> @@ -47,14 +47,17 @@ static int genl_family = -1;
>  
>  static void print_addr64(__u64 addr, char *buff, size_t len)
>  {
> -       __u16 *words = (__u16 *)&addr;
> +       union {
> +               __u64 w64;
> +               __u16 words[4];
> +       } id = { .w64 = addr };

This looks OK to me FWIW. Unions are commonly used to legalize aliasing,
so anybody looking at this will understand what's going on.

>         __u16 v;
>         int i, ret;
>         size_t written = 0;
>         char *sep = ":";
>  
>         for (i = 0; i < 4; i++) {
> -               v = ntohs(words[i]);
> +               v = ntohs(id.words[i]);
>  
>                 if (i == 3)
>                         sep = "";
> ..