Development discussion of WireGuard
 help / color / mirror / Atom feed
* Re: [PATCH] net: wireguard: avoid unused variable warning
       [not found] <20200505141327.746184-1-arnd@arndb.de>
@ 2020-05-05 20:06 ` Jason A. Donenfeld
  2020-05-05 20:51   ` Arnd Bergmann
  0 siblings, 1 reply; 2+ messages in thread
From: Jason A. Donenfeld @ 2020-05-05 20:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David S. Miller, Greg KH, Herbert Xu, Linux Crypto Mailing List,
	LKML, Netdev, WireGuard mailing list, clang-built-linux

On Tue, May 5, 2020 at 8:13 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> clang points out a harmless use of uninitialized variables that
> get passed into a local function but are ignored there:
>
> In file included from drivers/net/wireguard/ratelimiter.c:223:
> drivers/net/wireguard/selftest/ratelimiter.c:173:34: error: variable 'skb6' is uninitialized when used here [-Werror,-Wuninitialized]
>                 ret = timings_test(skb4, hdr4, skb6, hdr6, &test_count);
>                                                ^~~~
> drivers/net/wireguard/selftest/ratelimiter.c:123:29: note: initialize the variable 'skb6' to silence this warning
>         struct sk_buff *skb4, *skb6;
>                                    ^
>                                     = NULL
> drivers/net/wireguard/selftest/ratelimiter.c:173:40: error: variable 'hdr6' is uninitialized when used here [-Werror,-Wuninitialized]
>                 ret = timings_test(skb4, hdr4, skb6, hdr6, &test_count);
>                                                      ^~~~
> drivers/net/wireguard/selftest/ratelimiter.c:125:22: note: initialize the variable 'hdr6' to silence this warning
>         struct ipv6hdr *hdr6;
>                             ^

Seems like the code is a bit easier to read and is more uniform
looking by just initializing those two variables to NULL, like the
warning suggests. If you don't mind, I'll queue something up in my
tree to this effect.

By the way, I'm having a bit of a hard time reproducing the warning
with either clang-10 or clang-9. Just for my own curiosity, would you
mind sending the .config that results in this?

Jason

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] net: wireguard: avoid unused variable warning
  2020-05-05 20:06 ` [PATCH] net: wireguard: avoid unused variable warning Jason A. Donenfeld
@ 2020-05-05 20:51   ` Arnd Bergmann
  0 siblings, 0 replies; 2+ messages in thread
From: Arnd Bergmann @ 2020-05-05 20:51 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: David S. Miller, Greg KH, Herbert Xu, Linux Crypto Mailing List,
	LKML, Netdev, WireGuard mailing list, clang-built-linux

On Tue, May 5, 2020 at 10:07 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> On Tue, May 5, 2020 at 8:13 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > clang points out a harmless use of uninitialized variables that
> > get passed into a local function but are ignored there:
> >
> > In file included from drivers/net/wireguard/ratelimiter.c:223:
> > drivers/net/wireguard/selftest/ratelimiter.c:173:34: error: variable 'skb6' is uninitialized when used here [-Werror,-Wuninitialized]
> >                 ret = timings_test(skb4, hdr4, skb6, hdr6, &test_count);
> >                                                ^~~~
> > drivers/net/wireguard/selftest/ratelimiter.c:123:29: note: initialize the variable 'skb6' to silence this warning
> >         struct sk_buff *skb4, *skb6;
> >                                    ^
> >                                     = NULL
> > drivers/net/wireguard/selftest/ratelimiter.c:173:40: error: variable 'hdr6' is uninitialized when used here [-Werror,-Wuninitialized]
> >                 ret = timings_test(skb4, hdr4, skb6, hdr6, &test_count);
> >                                                      ^~~~
> > drivers/net/wireguard/selftest/ratelimiter.c:125:22: note: initialize the variable 'hdr6' to silence this warning
> >         struct ipv6hdr *hdr6;
> >                             ^
>
> Seems like the code is a bit easier to read and is more uniform
> looking by just initializing those two variables to NULL, like the
> warning suggests. If you don't mind, I'll queue something up in my
> tree to this effect.

I think we really should fix clang to not make this suggestion, as that
normally leads to worse code ;-)

The problem with initializing variables to NULL (or 0) is that it hides
real bugs when the NULL initialization end up being used in a place
where a non-NULL value is required, so I try hard not to send patches
that add those.

It's your code though, so if you prefer to do it that way, just do that
and add me as "Reported-by:"

> By the way, I'm having a bit of a hard time reproducing the warning
> with either clang-10 or clang-9. Just for my own curiosity, would you
> mind sending the .config that results in this?

See https://pastebin.com/6zRSUYax

       Arnd

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-05-06 10:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200505141327.746184-1-arnd@arndb.de>
2020-05-05 20:06 ` [PATCH] net: wireguard: avoid unused variable warning Jason A. Donenfeld
2020-05-05 20:51   ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).