Re: [PATCH] net: wireguard: avoid unused variable warning
On Tue, May 5, 2020 at 10:07 PM Jason A. Donenfeld wrote: > On Tue, May 5, 2020 at 8:13 AM Arnd Bergmann 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, _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, _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
Re: [PATCH] net: wireguard: avoid unused variable warning
On Tue, May 5, 2020 at 8:13 AM Arnd Bergmann 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, _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, _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
[PATCH] net: wireguard: avoid unused variable warning
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, _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, _count);
^~~~
drivers/net/wireguard/selftest/ratelimiter.c:125:22: note: initialize the
variable 'hdr6' to silence this warning
struct ipv6hdr *hdr6;
^
Shut up the warning by ensuring the variables are always initialized,
and make up for the loss of readability by changing the "#if IS_ENABLED()"
checks to regular "if (IS_ENABLED())".
Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
Signed-off-by: Arnd Bergmann
---
drivers/net/wireguard/selftest/ratelimiter.c | 32 +++-
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireguard/selftest/ratelimiter.c
b/drivers/net/wireguard/selftest/ratelimiter.c
index bcd6462e4540..f408b936e224 100644
--- a/drivers/net/wireguard/selftest/ratelimiter.c
+++ b/drivers/net/wireguard/selftest/ratelimiter.c
@@ -153,19 +153,22 @@ bool __init wg_ratelimiter_selftest(void)
skb_reset_network_header(skb4);
++test;
-#if IS_ENABLED(CONFIG_IPV6)
- skb6 = alloc_skb(sizeof(struct ipv6hdr), GFP_KERNEL);
- if (unlikely(!skb6)) {
- kfree_skb(skb4);
- goto err_nofree;
+ if (IS_ENABLED(CONFIG_IPV6)) {
+ skb6 = alloc_skb(sizeof(struct ipv6hdr), GFP_KERNEL);
+ if (unlikely(!skb6)) {
+ kfree_skb(skb4);
+ goto err_nofree;
+ }
+ skb6->protocol = htons(ETH_P_IPV6);
+ hdr6 = (struct ipv6hdr *)skb_put(skb6, sizeof(*hdr6));
+ hdr6->saddr.in6_u.u6_addr32[0] = htonl(1212);
+ hdr6->saddr.in6_u.u6_addr32[1] = htonl(289188);
+ skb_reset_network_header(skb6);
+ ++test;
+ } else {
+ skb6 = NULL;
+ hdr6 = NULL;
}
- skb6->protocol = htons(ETH_P_IPV6);
- hdr6 = (struct ipv6hdr *)skb_put(skb6, sizeof(*hdr6));
- hdr6->saddr.in6_u.u6_addr32[0] = htonl(1212);
- hdr6->saddr.in6_u.u6_addr32[1] = htonl(289188);
- skb_reset_network_header(skb6);
- ++test;
-#endif
for (trials = TRIALS_BEFORE_GIVING_UP;;) {
int test_count = 0, ret;
@@ -206,9 +209,8 @@ bool __init wg_ratelimiter_selftest(void)
err:
kfree_skb(skb4);
-#if IS_ENABLED(CONFIG_IPV6)
- kfree_skb(skb6);
-#endif
+ if (IS_ENABLED(CONFIG_IPV6))
+ kfree_skb(skb6);
err_nofree:
wg_ratelimiter_uninit();
wg_ratelimiter_uninit();
--
2.26.0
