[RFC] iproute2: ipila warning

[RFC] iproute2: ipila warning

[Stephen Hemminger]

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 };
        __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 = "";
..

Re: [RFC] iproute2: ipila warning

[Petr Machata]

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 = "";
> ..