linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: rtl8192x: fix bogus maybe-uninitialized warning
@ 2016-10-24 15:38 Arnd Bergmann
  2016-10-24 15:38 ` [PATCH] wireless: " Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-10-24 15:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-wireless, netdev, Arnd Bergmann, Kurt Kanzenbach, devel,
	linux-kernel

The rtllib_rx_extract_addr() is supposed to set up the mac addresses
for four possible cases, based on two bits of input data. For
some reason, gcc decides that it's possible that none of the these
four cases apply and the addresses remain uninitialized:

drivers/staging/rtl8192e/rtllib_rx.c: In function ‘rtllib_rx_InfraAdhoc’:
include/linux/etherdevice.h:316:61: error: ‘*((void *)&dst+4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/staging/rtl8192e/rtllib_rx.c:1318:5: note: ‘*((void *)&dst+4)’ was declared here
ded from /git/arm-soc/drivers/staging/rtl8192e/rtllib_rx.c:40:0:
include/linux/etherdevice.h:316:36: error: ‘dst’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/staging/rtl8192e/rtllib_rx.c:1318:5: note: ‘dst’ was declared here

This warning is clearly nonsense, but changing the last case into
'default' makes it obvious to the compiler too, which avoids the
warning and probably leads to better object code too.

As the same warning appears in other files that have the exact
same code, I'm fixing it in both rtl8192e and rtl8192u, even
though I did not observe it for the latter.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/rtl8192e/rtllib_rx.c                      | 2 +-
 drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 2 +-
 drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_rx.c b/drivers/staging/rtl8192e/rtllib_rx.c
index c743182b933e..d6777ecda64d 100644
--- a/drivers/staging/rtl8192e/rtllib_rx.c
+++ b/drivers/staging/rtl8192e/rtllib_rx.c
@@ -986,7 +986,7 @@ static void rtllib_rx_extract_addr(struct rtllib_device *ieee,
 		ether_addr_copy(src, hdr->addr4);
 		ether_addr_copy(bssid, ieee->current_network.bssid);
 		break;
-	case 0:
+	default:
 		ether_addr_copy(dst, hdr->addr1);
 		ether_addr_copy(src, hdr->addr2);
 		ether_addr_copy(bssid, hdr->addr3);
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index 6fa96d57d316..e68850897adf 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -553,7 +553,7 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 *hdr)
 		memcpy(hdr, hdr11->addr3, ETH_ALEN); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN); /* SA */
 		break;
-	case 0:
+	default:
 		memcpy(hdr, hdr11->addr1, ETH_ALEN); /* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN); /* SA */
 		break;
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 89cbc077a48d..2e4d2d7bc2e7 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -1079,7 +1079,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
 		memcpy(src, hdr->addr4, ETH_ALEN);
 		memcpy(bssid, ieee->current_network.bssid, ETH_ALEN);
 		break;
-	case 0:
+	default:
 		memcpy(dst, hdr->addr1, ETH_ALEN);
 		memcpy(src, hdr->addr2, ETH_ALEN);
 		memcpy(bssid, hdr->addr3, ETH_ALEN);
-- 
2.9.0

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

* [PATCH] wireless: fix bogus maybe-uninitialized warning
  2016-10-24 15:38 [PATCH] staging: rtl8192x: fix bogus maybe-uninitialized warning Arnd Bergmann
@ 2016-10-24 15:38 ` Arnd Bergmann
  2016-11-09  1:21   ` Kalle Valo
  2016-11-17  6:46   ` Kalle Valo
  0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-10-24 15:38 UTC (permalink / raw)
  To: Kalle Valo, Stanislav Yakovlev, Jouni Malinen, Johannes Berg,
	David S. Miller
  Cc: linux-wireless, netdev, Arnd Bergmann, linux-kernel

The hostap_80211_rx() function is supposed to set up the mac addresses
for four possible cases, based on two bits of input data. For
some reason, gcc decides that it's possible that none of the these
four cases apply and the addresses remain uninitialized:

drivers/net/wireless/intersil/hostap/hostap_80211_rx.c: In function ‘hostap_80211_rx’:
arch/x86/include/asm/string_32.h:77:14: warning: ‘src’ may be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/net/wireless/intel/ipw2x00/libipw_rx.c: In function ‘libipw_rx’:
arch/x86/include/asm/string_32.h:77:14: error: ‘dst’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/include/asm/string_32.h:78:22: error: ‘*((void *)&dst+4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This warning is clearly nonsense, but changing the last case into
'default' makes it obvious to the compiler too, which avoids the
warning and probably leads to better object code too.

The same code is duplicated several times in the kernel, so this
patch uses the same workaround for all copies. The exact configuration
was hit only very rarely in randconfig builds and I only saw it
in three drivers, but I assume that all of them are potentially
affected, and it's better to keep the code consistent.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/ath/ath6kl/wmi.c                  | 8 ++++----
 drivers/net/wireless/intel/ipw2x00/libipw_rx.c         | 2 +-
 drivers/net/wireless/intersil/hostap/hostap_80211_rx.c | 2 +-
 net/wireless/lib80211_crypt_tkip.c                     | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 3fd1cc98fd2f..84a6d12c3f8a 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -421,10 +421,6 @@ int ath6kl_wmi_dot11_hdr_remove(struct wmi *wmi, struct sk_buff *skb)
 
 	switch ((le16_to_cpu(wh.frame_control)) &
 		(IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS)) {
-	case 0:
-		memcpy(eth_hdr.h_dest, wh.addr1, ETH_ALEN);
-		memcpy(eth_hdr.h_source, wh.addr2, ETH_ALEN);
-		break;
 	case IEEE80211_FCTL_TODS:
 		memcpy(eth_hdr.h_dest, wh.addr3, ETH_ALEN);
 		memcpy(eth_hdr.h_source, wh.addr2, ETH_ALEN);
@@ -435,6 +431,10 @@ int ath6kl_wmi_dot11_hdr_remove(struct wmi *wmi, struct sk_buff *skb)
 		break;
 	case IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS:
 		break;
+	default:
+		memcpy(eth_hdr.h_dest, wh.addr1, ETH_ALEN);
+		memcpy(eth_hdr.h_source, wh.addr2, ETH_ALEN);
+		break;
 	}
 
 	skb_pull(skb, sizeof(struct ath6kl_llc_snap_hdr));
diff --git a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
index cef7f7d79cd9..1c1ec7bb9302 100644
--- a/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
+++ b/drivers/net/wireless/intel/ipw2x00/libipw_rx.c
@@ -507,7 +507,7 @@ int libipw_rx(struct libipw_device *ieee, struct sk_buff *skb,
 		memcpy(dst, hdr->addr3, ETH_ALEN);
 		memcpy(src, hdr->addr4, ETH_ALEN);
 		break;
-	case 0:
+	default:
 		memcpy(dst, hdr->addr1, ETH_ALEN);
 		memcpy(src, hdr->addr2, ETH_ALEN);
 		break;
diff --git a/drivers/net/wireless/intersil/hostap/hostap_80211_rx.c b/drivers/net/wireless/intersil/hostap/hostap_80211_rx.c
index 599f30f22841..34dbddbf3f9b 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_80211_rx.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_80211_rx.c
@@ -855,7 +855,7 @@ void hostap_80211_rx(struct net_device *dev, struct sk_buff *skb,
 		memcpy(dst, hdr->addr3, ETH_ALEN);
 		memcpy(src, hdr->addr4, ETH_ALEN);
 		break;
-	case 0:
+	default:
 		memcpy(dst, hdr->addr1, ETH_ALEN);
 		memcpy(src, hdr->addr2, ETH_ALEN);
 		break;
diff --git a/net/wireless/lib80211_crypt_tkip.c b/net/wireless/lib80211_crypt_tkip.c
index 71447cf86306..ba0a1f398ce5 100644
--- a/net/wireless/lib80211_crypt_tkip.c
+++ b/net/wireless/lib80211_crypt_tkip.c
@@ -556,7 +556,7 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 * hdr)
 		memcpy(hdr, hdr11->addr3, ETH_ALEN);	/* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN);	/* SA */
 		break;
-	case 0:
+	default:
 		memcpy(hdr, hdr11->addr1, ETH_ALEN);	/* DA */
 		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN);	/* SA */
 		break;
-- 
2.9.0

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

* Re: [PATCH] wireless: fix bogus maybe-uninitialized warning
  2016-10-24 15:38 ` [PATCH] wireless: " Arnd Bergmann
@ 2016-11-09  1:21   ` Kalle Valo
  2016-11-09  8:10     ` Johannes Berg
  2016-11-17  6:46   ` Kalle Valo
  1 sibling, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2016-11-09  1:21 UTC (permalink / raw)
  To: Arnd Bergmann, Johannes Berg
  Cc: Stanislav Yakovlev, Jouni Malinen, David S. Miller,
	linux-wireless, netdev, linux-kernel

Arnd Bergmann <arnd@arndb.de> writes:

> The hostap_80211_rx() function is supposed to set up the mac addresses
> for four possible cases, based on two bits of input data. For
> some reason, gcc decides that it's possible that none of the these
> four cases apply and the addresses remain uninitialized:
>
> drivers/net/wireless/intersil/hostap/hostap_80211_rx.c: In function ‘hostap_80211_rx’:
> arch/x86/include/asm/string_32.h:77:14: warning: ‘src’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/net/wireless/intel/ipw2x00/libipw_rx.c: In function ‘libipw_rx’:
> arch/x86/include/asm/string_32.h:77:14: error: ‘dst’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/include/asm/string_32.h:78:22: error: ‘*((void *)&dst+4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> This warning is clearly nonsense, but changing the last case into
> 'default' makes it obvious to the compiler too, which avoids the
> warning and probably leads to better object code too.
>
> The same code is duplicated several times in the kernel, so this
> patch uses the same workaround for all copies. The exact configuration
> was hit only very rarely in randconfig builds and I only saw it
> in three drivers, but I assume that all of them are potentially
> affected, and it's better to keep the code consistent.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/wireless/ath/ath6kl/wmi.c                  | 8 ++++----
>  drivers/net/wireless/intel/ipw2x00/libipw_rx.c         | 2 +-
>  drivers/net/wireless/intersil/hostap/hostap_80211_rx.c | 2 +-
>  net/wireless/lib80211_crypt_tkip.c                     | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)

[...]

> --- a/net/wireless/lib80211_crypt_tkip.c
> +++ b/net/wireless/lib80211_crypt_tkip.c
> @@ -556,7 +556,7 @@ static void michael_mic_hdr(struct sk_buff *skb, u8 * hdr)
>  		memcpy(hdr, hdr11->addr3, ETH_ALEN);	/* DA */
>  		memcpy(hdr + ETH_ALEN, hdr11->addr4, ETH_ALEN);	/* SA */
>  		break;
> -	case 0:
> +	default:
>  		memcpy(hdr, hdr11->addr1, ETH_ALEN);	/* DA */
>  		memcpy(hdr + ETH_ALEN, hdr11->addr2, ETH_ALEN);	/* SA */
>  		break;

Ideally we prefer that drivers/net/wireless and net/wireless changes are
split into different patches as they get applied to different trees.
Johannes, is it ok if I take this change through my tree this time?

-- 
Kalle Valo

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

* Re: [PATCH] wireless: fix bogus maybe-uninitialized warning
  2016-11-09  1:21   ` Kalle Valo
@ 2016-11-09  8:10     ` Johannes Berg
  2016-11-09 15:06       ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2016-11-09  8:10 UTC (permalink / raw)
  To: Kalle Valo, Arnd Bergmann
  Cc: Stanislav Yakovlev, Jouni Malinen, David S. Miller,
	linux-wireless, netdev, linux-kernel


> Ideally we prefer that drivers/net/wireless and net/wireless changes
> are
> split into different patches as they get applied to different trees.
> Johannes, is it ok if I take this change through my tree this time?

Sure, please do, thanks.

(I don't particularly care about the lib80211 stuff anyway)

johannes

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

* Re: [PATCH] wireless: fix bogus maybe-uninitialized warning
  2016-11-09  8:10     ` Johannes Berg
@ 2016-11-09 15:06       ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-11-09 15:06 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Kalle Valo, Stanislav Yakovlev, Jouni Malinen, David S. Miller,
	linux-wireless, netdev, linux-kernel

On Wednesday, November 9, 2016 9:10:46 AM CET Johannes Berg wrote:
> 
> > Ideally we prefer that drivers/net/wireless and net/wireless changes
> > are
> > split into different patches as they get applied to different trees.

Ok, good to know. I had split out the drivers/staging changes, but
wasn't sure about the granularity you'd want here.

> > Johannes, is it ok if I take this change through my tree this time?

> Sure, please do, thanks.
> 
> (I don't particularly care about the lib80211 stuff anyway)

Thanks!

	Arnd

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

* Re: wireless: fix bogus maybe-uninitialized warning
  2016-10-24 15:38 ` [PATCH] wireless: " Arnd Bergmann
  2016-11-09  1:21   ` Kalle Valo
@ 2016-11-17  6:46   ` Kalle Valo
  1 sibling, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2016-11-17  6:46 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Kalle Valo, Stanislav Yakovlev, Jouni Malinen, Johannes Berg,
	David S. Miller, linux-wireless, netdev, Arnd Bergmann,
	linux-kernel

Arnd Bergmann <arnd@arndb.de> wrote:
> The hostap_80211_rx() function is supposed to set up the mac addresses
> for four possible cases, based on two bits of input data. For
> some reason, gcc decides that it's possible that none of the these
> four cases apply and the addresses remain uninitialized:
> 
> drivers/net/wireless/intersil/hostap/hostap_80211_rx.c: In function ‘hostap_80211_rx’:
> arch/x86/include/asm/string_32.h:77:14: warning: ‘src’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/net/wireless/intel/ipw2x00/libipw_rx.c: In function ‘libipw_rx’:
> arch/x86/include/asm/string_32.h:77:14: error: ‘dst’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/include/asm/string_32.h:78:22: error: ‘*((void *)&dst+4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This warning is clearly nonsense, but changing the last case into
> 'default' makes it obvious to the compiler too, which avoids the
> warning and probably leads to better object code too.
> 
> The same code is duplicated several times in the kernel, so this
> patch uses the same workaround for all copies. The exact configuration
> was hit only very rarely in randconfig builds and I only saw it
> in three drivers, but I assume that all of them are potentially
> affected, and it's better to keep the code consistent.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied to wireless-drivers-next.git, thanks.

10f3366b4d89 wireless: fix bogus maybe-uninitialized warning

-- 
https://patchwork.kernel.org/patch/9392385/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

end of thread, other threads:[~2016-11-17  6:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-24 15:38 [PATCH] staging: rtl8192x: fix bogus maybe-uninitialized warning Arnd Bergmann
2016-10-24 15:38 ` [PATCH] wireless: " Arnd Bergmann
2016-11-09  1:21   ` Kalle Valo
2016-11-09  8:10     ` Johannes Berg
2016-11-09 15:06       ` Arnd Bergmann
2016-11-17  6:46   ` Kalle Valo

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).