Re: [mt76/mt7603/mac] Question about missing variable assignment
On 3/3/19 4:05 AM, Felix Fietkau wrote: > On 2019-03-02 22:10, Gustavo A. R. Silva wrote: >> Hi all, >> >> The following piece of code in >> drivers/net/wireless/mediatek/mt76/mt7603/mac.c >> is missing a variable assignment before line 1058. Notice that there >> is a potential execution path in which variable *i* is compared against >> magic number 15 at line 1075 without being initialized previously >> (this was reported by Coverity): >> >> 1055 out: >> 1056 final_rate_flags = info->status.rates[final_idx].flags; >> 1057 >> 1058 switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) { >> 1059 case MT_PHY_TYPE_CCK: >> 1060 cck = true; >> 1061 /* fall through */ >> 1062 case MT_PHY_TYPE_OFDM: >> 1063 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ) >> 1064 sband = &dev->mt76.sband_5g.sband; >> 1065 else >> 1066 sband = &dev->mt76.sband_2g.sband; >> 1067 final_rate &= GENMASK(5, 0); >> 1068 final_rate = mt7603_get_rate(dev, sband, final_rate, >> cck); >> 1069 final_rate_flags = 0; >> 1070 break; >> 1071 case MT_PHY_TYPE_HT_GF: >> 1072 case MT_PHY_TYPE_HT: >> 1073 final_rate_flags |= IEEE80211_TX_RC_MCS; >> 1074 final_rate &= GENMASK(5, 0); >> 1075 if (i > 15) >> 1076 return false; >> 1077 break; >> 1078 default: >> 1079 return false; >> 1080 } >> >> My guess is that such missing assignment should be something similar >> to the one at line 566: >> >> i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0); >> >> but I'm not sure what the proper arguments for macro FIELD_GET should >> be. >> >> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe >> >> What do you think? > Thanks for reporting this. The fix is simpler than that, the check > should be: if (final_rate > 15) > I will send a fix. > Great. Glad to help. :) Thanks -- Gustavo
[mt76/mt7603/mac] Question about missing variable assignment
Hi all, The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c is missing a variable assignment before line 1058. Notice that there is a potential execution path in which variable *i* is compared against magic number 15 at line 1075 without being initialized previously (this was reported by Coverity): 1055 out: 1056 final_rate_flags = info->status.rates[final_idx].flags; 1057 1058 switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) { 1059 case MT_PHY_TYPE_CCK: 1060 cck = true; 1061 /* fall through */ 1062 case MT_PHY_TYPE_OFDM: 1063 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ) 1064 sband = &dev->mt76.sband_5g.sband; 1065 else 1066 sband = &dev->mt76.sband_2g.sband; 1067 final_rate &= GENMASK(5, 0); 1068 final_rate = mt7603_get_rate(dev, sband, final_rate, cck); 1069 final_rate_flags = 0; 1070 break; 1071 case MT_PHY_TYPE_HT_GF: 1072 case MT_PHY_TYPE_HT: 1073 final_rate_flags |= IEEE80211_TX_RC_MCS; 1074 final_rate &= GENMASK(5, 0); 1075 if (i > 15) 1076 return false; 1077 break; 1078 default: 1079 return false; 1080 } My guess is that such missing assignment should be something similar to the one at line 566: i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0); but I'm not sure what the proper arguments for macro FIELD_GET should be. This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe What do you think? Thanks -- Gustavo
Re: -Wimplicit-fallthrough not working with ccache
Hi Kalle, On 2/16/19 5:21 AM, Kalle Valo wrote: > (replying to an old thread but renaming it) > > Kalle Valo writes: > >> "Gustavo A. R. Silva" wrote: >> >>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases >>> where we are expecting to fall through. >>> >>> Notice that in this particular case, I replaced "pass through" with >>> a proper "fall through" comment, which is what GCC is expecting >>> to find. >>> >>> Signed-off-by: Gustavo A. R. Silva >>> Signed-off-by: Kalle Valo >> >> Patch applied to ath-next branch of ath.git, thanks. >> >> f1d270ae10ff ath10k: htt_tx: mark expected switch fall-throughs > > Gustavo, I enabled W=1 on my ath10k build checks and it took me a while > to figure out why GCC was warning about fall through annotations missing > even I knew you had fixed them. Finally I figured out that the reason > was ccache, which I need because I work with different branches and need > to recompile the kernel quite often. > > If the plan is to enable -Wimplicit-fallthrough by default in the kernel > IMHO this might become an issue, as otherwise people using ccache start > seeing lots of invalid warnings. Apparently CCACHE_COMMENTS=1 will fix > that but my version of ccache doesn't support it, and how would everyone > learn that trick anyway? Or maybe CCACHE_COMMENTS can enabled through > kernel Makefile? > Can you share with me the warning messages you get? I just see the following warnings with linux-next: $ make CC="ccache gcc" W=1 drivers/net/wireless/ath/ath10k/htt_tx.o CC [M] drivers/net/wireless/ath/ath10k/htt_tx.o In file included from drivers/net/wireless/ath/ath10k/htt_tx.c:19: drivers/net/wireless/ath/ath10k/htt.h:1727:1: warning: alignment 1 of ‘struct ath10k_htt_txbuf_32’ is less than 4 [-Wpacked-not-aligned] } __packed; ^ drivers/net/wireless/ath/ath10k/htt.h:1734:1: warning: alignment 1 of ‘struct ath10k_htt_txbuf_64’ is less than 4 [-Wpacked-not-aligned] } __packed; ^ In my Makefile I have: KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough=3,) Thanks -- Gustavo
[PATCH] NFC: pn533: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. This patch fixes the following warnings: drivers/nfc/pn533/pn533.c: In function ‘pn533_transceive’: drivers/nfc/pn533/pn533.c:2142:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if (dev->tgt_active_prot == NFC_PROTO_FELICA) { ^ drivers/nfc/pn533/pn533.c:2150:2: note: here default: ^~~ drivers/nfc/pn533/pn533.c: In function ‘pn533_wq_mi_recv’: drivers/nfc/pn533/pn533.c:2267:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if (dev->tgt_active_prot == NFC_PROTO_FELICA) { ^ drivers/nfc/pn533/pn533.c:2276:2: note: here default: ^~~ Warning level 3 was used: -Wimplicit-fallthrough=3 This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough. Addresses-Coverity-ID: 1230487 ("Missing break in switch") Addresses-Coverity-ID: 1230488 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/nfc/pn533/pn533.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c index a0cc1cc45292..5961f14259e5 100644 --- a/drivers/nfc/pn533/pn533.c +++ b/drivers/nfc/pn533/pn533.c @@ -2147,6 +2147,7 @@ static int pn533_transceive(struct nfc_dev *nfc_dev, break; } + /* fall through */ default: /* jumbo frame ? */ if (skb->len > PN533_CMD_DATAEXCH_DATA_MAXLEN) { @@ -2273,6 +2274,7 @@ static void pn533_wq_mi_recv(struct work_struct *work) break; } + /* fall through */ default: skb_put_u8(skb, 1); /*TG*/ -- 2.20.1
[PATCH] NFC: st21nfca: Fix a couple of fall-through warnings
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. This patch fixes the following warnings by adding a missing break and a fall-through annotation: drivers/nfc/st21nfca/dep.c: In function ‘st21nfca_tm_event_send_data’: drivers/nfc/st21nfca/dep.c:391:3: warning: this statement may fall through [-Wimplicit-fallthrough=] switch (cmd1) { ^~ drivers/nfc/st21nfca/dep.c:404:2: note: here default: ^~~ In file included from ./include/linux/kernel.h:15, from ./include/linux/skbuff.h:17, from ./include/net/nfc/hci.h:21, from drivers/nfc/st21nfca/dep.c:17: drivers/nfc/st21nfca/dep.c: In function ‘st21nfca_im_recv_dep_res_cb’: ./include/linux/printk.h:303:2: warning: this statement may fall through [-Wimplicit-fallthrough=] printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__) ^~~ drivers/nfc/st21nfca/dep.c:622:4: note: in expansion of macro ‘pr_err’ pr_err("Received a ACK/NACK PDU\n"); ^~ drivers/nfc/st21nfca/dep.c:623:3: note: here case ST21NFCA_NFC_DEP_PFB_I_PDU: ^~~~ Warning level 3 was used: -Wimplicit-fallthrough=3 This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough. Signed-off-by: Gustavo A. R. Silva --- drivers/nfc/st21nfca/dep.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/nfc/st21nfca/dep.c b/drivers/nfc/st21nfca/dep.c index 3420c5104c94..60f013063f80 100644 --- a/drivers/nfc/st21nfca/dep.c +++ b/drivers/nfc/st21nfca/dep.c @@ -401,6 +401,7 @@ static int st21nfca_tm_event_send_data(struct nfc_hci_dev *hdev, default: return 1; } + break; default: return 1; } @@ -620,6 +621,7 @@ static void st21nfca_im_recv_dep_res_cb(void *context, struct sk_buff *skb, switch (ST21NFCA_NFC_DEP_PFB_TYPE(dep_res->pfb)) { case ST21NFCA_NFC_DEP_PFB_ACK_NACK_PDU: pr_err("Received a ACK/NACK PDU\n"); + /* fall through */ case ST21NFCA_NFC_DEP_PFB_I_PDU: info->dep_info.curr_nfc_dep_pni = ST21NFCA_NFC_DEP_PFB_PNI(dep_res->pfb + 1); -- 2.20.1
Re: [PATCH] iwlwifi: eeprom-parse: use struct_size() in kzalloc()
On 1/22/19 4:58 AM, Luciano Coelho wrote: > On Tue, 2019-01-08 at 11:17 -0600, Gustavo A. R. Silva wrote: >> One of the more common cases of allocation size calculations is >> finding the >> size of a structure that has a zero-sized array at the end, along >> with memory >> for some number of elements for that array. For example: >> >> struct foo { >> int stuff; >> void *entry[]; >> }; >> >> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, >> GFP_KERNEL); >> >> Instead of leaving these open-coded and prone to type mistakes, we >> can now >> use the new struct_size() helper: >> >> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); >> >> This code was detected with the help of Coccinelle. >> >> Signed-off-by: Gustavo A. R. Silva >> --- > > Applied to our internal tree, thanks! It will reach the mainline > following our normal upstreaming process. > Great. :) Thanks, Luca. -- Gustavo
[PATCH][next] cfg80211: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. This patch fixes the following warnings: net/wireless/wext-compat.c:1327:6: warning: this statement may fall through [-Wimplicit-fallthrough=] net/wireless/wext-compat.c:1341:6: warning: this statement may fall through [-Wimplicit-fallthrough=] Warning level 3 was used: -Wimplicit-fallthrough=3 This patch is part of the ongoing efforts to enabling -Wimplicit-fallthrough Signed-off-by: Gustavo A. R. Silva --- net/wireless/wext-compat.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c index 06943d9c9835..d522787c7354 100644 --- a/net/wireless/wext-compat.c +++ b/net/wireless/wext-compat.c @@ -1337,6 +1337,7 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev) wstats.qual.qual = sig + 110; break; } + /* fall through */ case CFG80211_SIGNAL_TYPE_UNSPEC: if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_SIGNAL)) { wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED; @@ -1345,6 +1346,7 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev) wstats.qual.qual = sinfo.signal; break; } + /* fall through */ default: wstats.qual.updated |= IW_QUAL_LEVEL_INVALID; wstats.qual.updated |= IW_QUAL_QUAL_INVALID; -- 2.20.1
[PATCH][next] iwlwifi: mvm: use struct_size() in kzalloc()
One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct foo { int stuff; struct boo entry[]; }; instance = kzalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL); Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper: instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c index d9afedc3d1d9..a2b2bdc5cd09 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c @@ -947,14 +947,12 @@ iwl_parse_nvm_data(struct iwl_trans *trans, const struct iwl_cfg *cfg, const __le16 *ch_section; if (cfg->nvm_type != IWL_NVM_EXT) - data = kzalloc(sizeof(*data) + - sizeof(struct ieee80211_channel) * - IWL_NVM_NUM_CHANNELS, + data = kzalloc(struct_size(data, channels, + IWL_NVM_NUM_CHANNELS), GFP_KERNEL); else - data = kzalloc(sizeof(*data) + - sizeof(struct ieee80211_channel) * - IWL_NVM_NUM_CHANNELS_EXT, + data = kzalloc(struct_size(data, channels, + IWL_NVM_NUM_CHANNELS_EXT), GFP_KERNEL); if (!data) return NULL; @@ -1444,9 +1442,7 @@ struct iwl_nvm_data *iwl_get_nvm(struct iwl_trans *trans, if (empty_otp) IWL_INFO(trans, "OTP is empty\n"); - nvm = kzalloc(sizeof(*nvm) + - sizeof(struct ieee80211_channel) * IWL_NUM_CHANNELS, - GFP_KERNEL); + nvm = kzalloc(struct_size(nvm, channels, IWL_NUM_CHANNELS), GFP_KERNEL); if (!nvm) { ret = -ENOMEM; goto out; -- 2.20.1
[PATCH] iwlwifi: nvm-parse: use struct_size() in kzalloc()
One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct foo { int stuff; void *entry[]; }; instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL); Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper: instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva --- .../net/wireless/intel/iwlwifi/iwl-nvm-parse.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c index d9afedc3d1d9..8c720b42dc36 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c @@ -947,15 +947,13 @@ iwl_parse_nvm_data(struct iwl_trans *trans, const struct iwl_cfg *cfg, const __le16 *ch_section; if (cfg->nvm_type != IWL_NVM_EXT) - data = kzalloc(sizeof(*data) + - sizeof(struct ieee80211_channel) * - IWL_NVM_NUM_CHANNELS, - GFP_KERNEL); + data = kzalloc(struct_size(data, channels, + IWL_NVM_NUM_CHANNELS), + GFP_KERNEL); else - data = kzalloc(sizeof(*data) + - sizeof(struct ieee80211_channel) * - IWL_NVM_NUM_CHANNELS_EXT, - GFP_KERNEL); + data = kzalloc(struct_size(data, channels, + IWL_NVM_NUM_CHANNELS_EXT), + GFP_KERNEL); if (!data) return NULL; @@ -1444,9 +1442,7 @@ struct iwl_nvm_data *iwl_get_nvm(struct iwl_trans *trans, if (empty_otp) IWL_INFO(trans, "OTP is empty\n"); - nvm = kzalloc(sizeof(*nvm) + - sizeof(struct ieee80211_channel) * IWL_NUM_CHANNELS, - GFP_KERNEL); + nvm = kzalloc(struct_size(nvm, channels, IWL_NUM_CHANNELS), GFP_KERNEL); if (!nvm) { ret = -ENOMEM; goto out; -- 2.20.1
[PATCH] iwlwifi: eeprom-parse: use struct_size() in kzalloc()
One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct foo { int stuff; void *entry[]; }; instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL); Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper: instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c b/drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c index 75940ac406b9..04338c3a6205 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-eeprom-parse.c @@ -850,8 +850,7 @@ iwl_parse_eeprom_data(struct device *dev, const struct iwl_cfg *cfg, if (WARN_ON(!cfg || !cfg->eeprom_params)) return NULL; - data = kzalloc(sizeof(*data) + - sizeof(struct ieee80211_channel) * IWL_NUM_CHANNELS, + data = kzalloc(struct_size(data, channels, IWL_NUM_CHANNELS), GFP_KERNEL); if (!data) return NULL; -- 2.20.1
[PATCH] qtnfmac: use struct_size() in kzalloc()
One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct foo { int stuff; void *entry[]; }; instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL); Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper: instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL); This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/quantenna/qtnfmac/commands.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c index 659e7649fe22..cf386f579060 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/commands.c +++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c @@ -914,9 +914,8 @@ qtnf_cmd_resp_proc_hw_info(struct qtnf_bus *bus, if (WARN_ON(resp->n_reg_rules > NL80211_MAX_SUPP_REG_RULES)) return -E2BIG; - hwinfo->rd = kzalloc(sizeof(*hwinfo->rd) -+ sizeof(struct ieee80211_reg_rule) -* resp->n_reg_rules, GFP_KERNEL); + hwinfo->rd = kzalloc(struct_size(hwinfo->rd, reg_rules, +resp->n_reg_rules), GFP_KERNEL); if (!hwinfo->rd) return -ENOMEM; -- 2.20.1
Re: [PATCH] nfc: af_nfc: Fix Spectre v1 vulnerability
On 12/22/18 8:42 PM, David Miller wrote: From: "Gustavo A. R. Silva" Date: Sat, 22 Dec 2018 17:37:35 -0600 I wonder if you can take this one too: https://lore.kernel.org/lkml/20181221212229.GA32635@embeddedor/ It's pretty similar to the af_nfc one. Sure, done. Great. Thanks. -- Gustavo
Re: [PATCH] nfc: af_nfc: Fix Spectre v1 vulnerability
On 12/22/18 5:09 PM, David Miller wrote: From: "Gustavo A. R. Silva" Date: Fri, 21 Dec 2018 15:47:53 -0600 proto is indirectly controlled by user-space, hence leading to a potential exploitation of the Spectre variant 1 vulnerability. This issue was detected with the help of Smatch: net/nfc/af_nfc.c:42 nfc_sock_create() warn: potential spectre issue 'proto_tab' [w] (local cap) Fix this by sanitizing proto before using it to index proto_tab. Notice that given that speculation windows are large, the policy is to kill the speculation on the first load and not worry if it can be completed with a dependent load/store [1]. [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2 Signed-off-by: Gustavo A. R. Silva I'll take this directly, and queued up for -stable. Dave, I wonder if you can take this one too: https://lore.kernel.org/lkml/20181221212229.GA32635@embeddedor/ It's pretty similar to the af_nfc one. Thanks -- Gustavo
[PATCH] nfc: af_nfc: Fix Spectre v1 vulnerability
proto is indirectly controlled by user-space, hence leading to a potential exploitation of the Spectre variant 1 vulnerability. This issue was detected with the help of Smatch: net/nfc/af_nfc.c:42 nfc_sock_create() warn: potential spectre issue 'proto_tab' [w] (local cap) Fix this by sanitizing proto before using it to index proto_tab. Notice that given that speculation windows are large, the policy is to kill the speculation on the first load and not worry if it can be completed with a dependent load/store [1]. [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2 Signed-off-by: Gustavo A. R. Silva --- net/nfc/af_nfc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/nfc/af_nfc.c b/net/nfc/af_nfc.c index d3e594eb36d0..256f3c57059e 100644 --- a/net/nfc/af_nfc.c +++ b/net/nfc/af_nfc.c @@ -21,6 +21,7 @@ #include #include +#include #include "nfc.h" @@ -37,6 +38,7 @@ static int nfc_sock_create(struct net *net, struct socket *sock, int proto, if (proto < 0 || proto >= NFC_SOCKPROTO_MAX) return -EINVAL; + proto = array_index_nospec(proto, NFC_SOCKPROTO_MAX); read_lock(&proto_tab_lock); if (proto_tab[proto] && try_module_get(proto_tab[proto]->owner)) { -- 2.20.1
Re: linux-next: build warnings after merge of the wireless-drivers-next tree
On 12/3/18 6:38 AM, Luciano Coelho wrote: On Fri, 2018-11-30 at 09:43 +0200, Kalle Valo wrote: Stephen Rothwell writes: Hi Kalle, On Fri, 30 Nov 2018 06:33:47 +0200 Kalle Valo wrote: I take it that -Wimplict-fallthrough is not enabled by default yet? So Dave and Linus won't see these warnings? Correct. What I'm planning is to send a pull request to Dave today and fix these warnings in the next pull request. Does that sound good? That sounds fine. Great, thanks for the confirmation. I'll then send the pull request today. Just FYI, I enabled this by default in our internal tree so we will notice when it happens before sending the patch upstream. That's really great. Thanks, Luca -- Gustavo
Re: [PATCH v2] wireless: mark expected switch fall-throughs
On 10/23/18 10:59 AM, Gustavo A. R. Silva wrote: > > On 10/23/18 9:01 AM, Johannes Berg wrote: >> On Tue, 2018-10-23 at 02:13 +0200, Gustavo A. R. Silva wrote: >>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases >>> where we are expecting to fall through. >>> >>> Warning level 3 was used: -Wimplicit-fallthrough=3 >>> >>> This code was not tested and GCC 7.2.0 was used to compile it. >> >> Look, I'm not going to make this any clearer: I'm not applying patches >> like that where you've invested no effort whatsoever on verifying that >> they're correct. >> > > How do you suggest me to verify that every part is correct in this type > of patches? > BTW... I'm under the impression you think that I don't even look at the code. Is that correct? I've been working on this for quite a while, and in every case I try to understand the code in terms of the context in which every warning is reported. I look for dependencies between variables in adjacent switch cases, that could make think it might be OK for some of those cases to fall through to the one below. I check the names of the case labels to see if there might be any relation between them, or if they are totally diferent as it might be the case for labels that indicate the transmision(FOO_TX) or reception of data(FOO_RX). Something similar to the latter is the case with on/off logic (FOO_ON/FOO_OFF). These are some of the things I review in the code, so I can have an idea if the warning is a false positive or an actual bug. Here is a bug I found yesterday at drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 5690 case WLAN_CIPHER_SUITE_CCMP: 5691 key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX; 5692 break; 5693 case WLAN_CIPHER_SUITE_TKIP: 5694 key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC; 5695 default: 5696 return -EOPNOTSUPP; 5697 } Notice how the absence of a break statement is very suspicious in case WLAN_CIPHER_SUITE_TKIP, because the code for case WLAN_CIPHER_SUITE_CCMP is pretty similar and in that case there is a break at the bottom. Now, that's not the only thing that looks supicious: in the absence of a break, the code falls through to the default case, which returns the error value -EOPNOTSUPP. So, even when key->flags is updated, the code always returns an error for case WLAN_CIPHER_SUITE_TKIP. This analysis led me to think that this is an actual bug, so I sent a patch to fix it: https://lore.kernel.org/patchwork/patch/1002440/ Notice that this bug has been there since 2015: commit 26f1fad29ad973b0fb26a9ca3dcb2a73dde781aa Now, let's take a look at the following warning in net/wireless/chan.c: net/wireless/chan.c: In function ‘cfg80211_chandef_usable’: net/wireless/chan.c:748:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if (!ht_cap->ht_supported) ^ net/wireless/chan.c:750:2: note: here case NL80211_CHAN_WIDTH_20_NOHT: ^~~~ This is the piece of code at net/wireless/chan.c:740: 740 case NL80211_CHAN_WIDTH_5: 741 width = 5; 742 break; 743 case NL80211_CHAN_WIDTH_10: 744 prohibited_flags |= IEEE80211_CHAN_NO_10MHZ; 745 width = 10; 746 break; 747 case NL80211_CHAN_WIDTH_20: 748 if (!ht_cap->ht_supported) 749 return false; 750 case NL80211_CHAN_WIDTH_20_NOHT: 751 prohibited_flags |= IEEE80211_CHAN_NO_20MHZ; 752 width = 20; 753 break; 754 case NL80211_CHAN_WIDTH_40: 755 width = 40; 756 if (!ht_cap->ht_supported) 757 return false; 758 if (!(ht_cap->cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) || 759 ht_cap->cap & IEEE80211_HT_CAP_40MHZ_INTOLERANT) 760 return false; 761 if (chandef->center_freq1 < control_freq && 762 chandef->chan->flags & IEEE80211_CHAN_NO_HT40MINUS) 763 return false; 764 if (chandef->center_freq1 > control_freq && 765 chandef->chan->flags & IEEE80211_CHAN_NO_HT40PLUS) 766 return false; 767 break; Notice that the warning was reported at line 748, but I'm including more code here to make it explicitly clear that I not only focus my atention in a very narrowed piece of code around the warning. Now, I don't see anything supicious. The labels NL80211_CHAN_WIDTH_20 and NL80211_CHAN_WIDTH_20_NOHT seem to be related, and it's even less supicious when there
Re: [PATCH v2] wireless: mark expected switch fall-throughs
On 10/23/18 9:01 AM, Johannes Berg wrote: > On Tue, 2018-10-23 at 02:13 +0200, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases >> where we are expecting to fall through. >> >> Warning level 3 was used: -Wimplicit-fallthrough=3 >> >> This code was not tested and GCC 7.2.0 was used to compile it. > > Look, I'm not going to make this any clearer: I'm not applying patches > like that where you've invested no effort whatsoever on verifying that > they're correct. > How do you suggest me to verify that every part is correct in this type of patches? Thanks -- Gustavo
Re: [PATCH] wireless: mark expected switch fall-throughs
On 7/6/18 2:29 PM, Johannes Berg wrote: > Hi Gustavo, > >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases >> where we are expecting to fall through. > > You dropped the remark saying you didn't review them, but did you? > I'll add it in v2. >> case NL80211_CHAN_WIDTH_20: >> if (!ht_cap->ht_supported) >> return false; >> +/* else: fall through */ > > What's the point in else:? > > We also don't necessarily write > > if (!...) > return false; > else > do_something(); > > but rather > > if (!...) > return false; > do_something(). > > I think I'd prefer without the "else:" > Sure thing. I'll change this in v2. I'll send v2 shortly. Thanks for the feedback. -- Gustavo
[PATCH v2] wireless: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Warning level 3 was used: -Wimplicit-fallthrough=3 This code was not tested and GCC 7.2.0 was used to compile it. Signed-off-by: Gustavo A. R. Silva --- Changes in v2: - Explicity mention in the commit log that this the code was not tested. - Use warning level 3 instead of 2. - Change the form "else: fall through" to "fall through" instead. - Address -Wimplicit-fallthrough warnings in net/wireless/scan.c net/wireless/chan.c| 2 ++ net/wireless/nl80211.c | 9 + net/wireless/scan.c| 2 +- net/wireless/wext-compat.c | 2 ++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/net/wireless/chan.c b/net/wireless/chan.c index 2db713d..0a45265 100644 --- a/net/wireless/chan.c +++ b/net/wireless/chan.c @@ -747,6 +747,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, case NL80211_CHAN_WIDTH_20: if (!ht_cap->ht_supported) return false; + /* fall through */ case NL80211_CHAN_WIDTH_20_NOHT: prohibited_flags |= IEEE80211_CHAN_NO_20MHZ; width = 20; @@ -769,6 +770,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK; if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ) return false; + /* fall through */ case NL80211_CHAN_WIDTH_80: if (!vht_cap->vht_supported) return false; diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 744b585..0276370 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -1706,6 +1706,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 1: if (nla_put(msg, NL80211_ATTR_CIPHER_SUITES, sizeof(u32) * rdev->wiphy.n_cipher_suites, @@ -1752,6 +1753,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 2: if (nl80211_put_iftypes(msg, NL80211_ATTR_SUPPORTED_IFTYPES, rdev->wiphy.interface_modes)) @@ -1759,6 +1761,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 3: nl_bands = nla_nest_start(msg, NL80211_ATTR_WIPHY_BANDS); if (!nl_bands) @@ -1784,6 +1787,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->chan_start++; if (state->split) break; + /* fall through */ default: /* add frequencies */ nl_freqs = nla_nest_start( @@ -1837,6 +1841,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 4: nl_cmds = nla_nest_start(msg, NL80211_ATTR_SUPPORTED_COMMANDS); if (!nl_cmds) @@ -1863,6 +1868,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 5: if (rdev->ops->remain_on_channel && (rdev->wiphy.flags & WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL) && @@ -1880,6 +1886,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 6: #ifdef CONFIG_PM if (nl80211_send_wowlan(msg, rdev, state->split)) @@ -1890,6 +1897,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, #else state->split_start++; #endif + /* fall through */ case 7: if (nl80211_put_iftypes(msg, NL80211_ATTR_SOFTWARE_IFTYPES, rdev->wiphy.software_iftypes)) @@ -1902,6 +1910,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
[PATCH 17/20] rt2x00: rt61pci: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ralink/rt2x00/rt61pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ralink/rt2x00/rt61pci.c b/drivers/net/wireless/ralink/rt2x00/rt61pci.c index cb0e119..4c5de8f 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt61pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt61pci.c @@ -2226,7 +2226,7 @@ static void rt61pci_txdone(struct rt2x00_dev *rt2x00dev) break; case 6: /* Failure, excessive retries */ __set_bit(TXDONE_EXCESSIVE_RETRY, &txdesc.flags); - /* Don't break, this is a failed frame! */ + /* Fall through - this is a failed frame! */ default: /* Failure */ __set_bit(TXDONE_FAILURE, &txdesc.flags); } -- 2.7.4
[PATCH 19/20] rtlwifi: rtl8821ae: phy: Mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c index 176deb2..a75451c 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c @@ -394,6 +394,7 @@ static void _rtl8812ae_phy_set_rfe_reg_24g(struct ieee80211_hw *hw) rtl_set_bbreg(hw, RB_RFE_INV, BMASKRFEINV, 0x000); break; } + /* fall through */ case 0: case 2: default: -- 2.7.4
[PATCH 06/20] carl9170: tx: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/carl9170/tx.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c index 8c75651..2407931 100644 --- a/drivers/net/wireless/ath/carl9170/tx.c +++ b/drivers/net/wireless/ath/carl9170/tx.c @@ -830,10 +830,12 @@ static bool carl9170_tx_rts_check(struct ar9170 *ar, case CARL9170_ERP_AUTO: if (ampdu) break; + /* fall through */ case CARL9170_ERP_MAC80211: if (!(rate->flags & IEEE80211_TX_RC_USE_RTS_CTS)) break; + /* fall through */ case CARL9170_ERP_RTS: if (likely(!multi)) @@ -854,6 +856,7 @@ static bool carl9170_tx_cts_check(struct ar9170 *ar, case CARL9170_ERP_MAC80211: if (!(rate->flags & IEEE80211_TX_RC_USE_CTS_PROTECT)) break; + /* fall through */ case CARL9170_ERP_CTS: return true; -- 2.7.4
[PATCH 12/20] prism54: islpci_dev: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 114947 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/intersil/prism54/islpci_dev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/intersil/prism54/islpci_dev.c b/drivers/net/wireless/intersil/prism54/islpci_dev.c index 325176d..ad6d3a5 100644 --- a/drivers/net/wireless/intersil/prism54/islpci_dev.c +++ b/drivers/net/wireless/intersil/prism54/islpci_dev.c @@ -932,6 +932,7 @@ islpci_set_state(islpci_private *priv, islpci_state_t new_state) switch (new_state) { case PRV_STATE_OFF: priv->state_off++; + /* fall through */ default: priv->state = new_state; break; -- 2.7.4
[PATCH 11/20] prism54: isl_ioctl: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/intersil/prism54/isl_ioctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/intersil/prism54/isl_ioctl.c b/drivers/net/wireless/intersil/prism54/isl_ioctl.c index 334717b0..3ccf2a4 100644 --- a/drivers/net/wireless/intersil/prism54/isl_ioctl.c +++ b/drivers/net/wireless/intersil/prism54/isl_ioctl.c @@ -1691,6 +1691,7 @@ static int prism54_get_encodeext(struct net_device *ndev, case DOT11_AUTH_BOTH: case DOT11_AUTH_SK: wrqu->encoding.flags |= IW_ENCODE_RESTRICTED; + /* fall through */ case DOT11_AUTH_OS: default: wrqu->encoding.flags |= IW_ENCODE_OPEN; -- 2.7.4
[PATCH 14/20] rt2x00: rt2400pci: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ralink/rt2x00/rt2400pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c index 0bc8b02..49a7327 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c @@ -1302,7 +1302,7 @@ static void rt2400pci_txdone(struct rt2x00_dev *rt2x00dev, break; case 2: /* Failure, excessive retries */ __set_bit(TXDONE_EXCESSIVE_RETRY, &txdesc.flags); - /* Don't break, this is a failed frame! */ + /* Fall through - this is a failed frame! */ default: /* Failure */ __set_bit(TXDONE_FAILURE, &txdesc.flags); } -- 2.7.4
[PATCH 09/20] orinoco_usb: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c index 21bb684..40a8b941 100644 --- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c +++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c @@ -908,6 +908,7 @@ static int ezusb_access_ltv(struct ezusb_priv *upriv, case EZUSB_CTX_REQ_SUBMITTED: if (!ctx->in_rid) break; + /* fall through */ default: err("%s: Unexpected context state %d", __func__, state); -- 2.7.4
[PATCH 07/20] iwlegacy: 4965-mac: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c b/drivers/net/wireless/intel/iwlegacy/4965-mac.c index 280cd8a..6b4488a 100644 --- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c +++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c @@ -559,7 +559,7 @@ il4965_translate_rx_status(struct il_priv *il, u32 decrypt_in) decrypt_out |= RX_RES_STATUS_BAD_KEY_TTAK; break; } - /* fall through if TTAK OK */ + /* fall through - if TTAK OK */ default: if (!(decrypt_in & RX_MPDU_RES_STATUS_ICV_OK)) decrypt_out |= RX_RES_STATUS_BAD_ICV_MIC; -- 2.7.4
[PATCH 05/20] carl9170: rx: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1056534 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/carl9170/rx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c index 7050632..f7c2f19 100644 --- a/drivers/net/wireless/ath/carl9170/rx.c +++ b/drivers/net/wireless/ath/carl9170/rx.c @@ -766,6 +766,7 @@ static void carl9170_rx_untie_data(struct ar9170 *ar, u8 *buf, int len) goto drop; } + /* fall through */ case AR9170_RX_STATUS_MPDU_MIDDLE: /* These are just data + mac status */ -- 2.7.4
[PATCH 02/20] ath9k: ar5008_phy: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1056532 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath9k/ar5008_phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath9k/ar5008_phy.c b/drivers/net/wireless/ath/ath9k/ar5008_phy.c index 11d6f975..dae9540 100644 --- a/drivers/net/wireless/ath/ath9k/ar5008_phy.c +++ b/drivers/net/wireless/ath/ath9k/ar5008_phy.c @@ -586,7 +586,7 @@ static void ar5008_hw_init_chain_masks(struct ath_hw *ah) REG_WRITE(ah, AR_PHY_CAL_CHAINMASK, 0x7); break; } - /* else: fall through */ + /* fall through */ case 0x1: case 0x2: case 0x7: -- 2.7.4
[PATCH 00/20] Mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, this patchset aims to mark switch cases where we are expecting to fall through. Thanks! Gustavo A. R. Silva (20): ath6kl: Mark expected switch fall-through ath9k: mark expected switch fall-through ath9k: ar9002_phy: mark expected switch fall-throughs ath9k_hw: mark expected switch fall-through carl9170: rx: mark expected switch fall-through carl9170: tx: mark expected switch fall-throughs iwlegacy: 4965-mac: mark expected switch fall-through iwlegacy: common: mark expected switch fall-throughs orinoco_usb: mark expected switch fall-through prism54: isl_38xx: Mark expected switch fall-through prism54: isl_ioctl: mark expected switch fall-through prism54: islpci_dev: mark expected switch fall-through mwifiex: Mark expected switch fall-through rt2x00: rt2400pci: mark expected switch fall-through rt2x00: rt2500pci: mark expected switch fall-through rt2x00: rt2800lib: mark expected switch fall-through rt2x00: rt61pci: mark expected switch fall-through ray_cs: mark expected switch fall-throughs rtlwifi: rtl8821ae: phy: Mark expected switch fall-through zd1201: mark expected switch fall-through drivers/net/wireless/ath/ath6kl/main.c | 1 + drivers/net/wireless/ath/ath9k/ar5008_phy.c | 2 +- drivers/net/wireless/ath/ath9k/ar9002_phy.c | 2 +- drivers/net/wireless/ath/ath9k/hw.c | 1 + drivers/net/wireless/ath/carl9170/rx.c | 1 + drivers/net/wireless/ath/carl9170/tx.c | 3 +++ drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- drivers/net/wireless/intel/iwlegacy/common.c | 2 ++ drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 1 + drivers/net/wireless/intersil/prism54/isl_38xx.c | 1 + drivers/net/wireless/intersil/prism54/isl_ioctl.c| 1 + drivers/net/wireless/intersil/prism54/islpci_dev.c | 1 + drivers/net/wireless/marvell/mwifiex/ie.c| 1 + drivers/net/wireless/ralink/rt2x00/rt2400pci.c | 2 +- drivers/net/wireless/ralink/rt2x00/rt2500pci.c | 2 +- drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 4 drivers/net/wireless/ralink/rt2x00/rt61pci.c | 2 +- drivers/net/wireless/ray_cs.c| 2 ++ drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c | 1 + drivers/net/wireless/zydas/zd1201.c | 1 + 20 files changed, 27 insertions(+), 6 deletions(-) -- 2.7.4
[PATCH 20/20] zd1201: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/zydas/zd1201.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/zydas/zd1201.c b/drivers/net/wireless/zydas/zd1201.c index 2534038..22c70f1 100644 --- a/drivers/net/wireless/zydas/zd1201.c +++ b/drivers/net/wireless/zydas/zd1201.c @@ -969,6 +969,7 @@ static int zd1201_set_mode(struct net_device *dev, */ zd1201_join(zd, "\0-*#\0", 5); /* Put port in pIBSS */ + /* Fall through */ case 8: /* No pseudo-IBSS in wireless extensions (yet) */ porttype = ZD1201_PORTTYPE_PSEUDOIBSS; break; -- 2.7.4
[PATCH 18/20] ray_cs: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 114948 ("Missing break in switch") Addresses-Coverity-ID: 114949 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ray_cs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c index 08c607c..33ad875 100644 --- a/drivers/net/wireless/ray_cs.c +++ b/drivers/net/wireless/ray_cs.c @@ -889,8 +889,10 @@ static int ray_hw_xmit(unsigned char *data, int len, struct net_device *dev, switch (ccsindex = get_free_tx_ccs(local)) { case ECCSBUSY: pr_debug("ray_hw_xmit tx_ccs table busy\n"); + /* fall through */ case ECCSFULL: pr_debug("ray_hw_xmit No free tx ccs\n"); + /* fall through */ case ECARDGONE: netif_stop_queue(dev); return XMIT_NO_CCS; -- 2.7.4
[PATCH 16/20] rt2x00: rt2800lib: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 145198 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c index 9e7b893..0e9 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c @@ -2482,6 +2482,7 @@ static void rt2800_config_channel_rf3052(struct rt2x00_dev *rt2x00dev, switch (rt2x00dev->default_ant.tx_chain_num) { case 1: rt2x00_set_field8(&rfcsr, RFCSR1_TX1_PD, 1); + /* fall through */ case 2: rt2x00_set_field8(&rfcsr, RFCSR1_TX2_PD, 1); break; @@ -2490,6 +2491,7 @@ static void rt2800_config_channel_rf3052(struct rt2x00_dev *rt2x00dev, switch (rt2x00dev->default_ant.rx_chain_num) { case 1: rt2x00_set_field8(&rfcsr, RFCSR1_RX1_PD, 1); + /* fall through */ case 2: rt2x00_set_field8(&rfcsr, RFCSR1_RX2_PD, 1); break; @@ -9457,8 +9459,10 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev) switch (rx_chains) { case 3: spec->ht.mcs.rx_mask[2] = 0xff; + /* fall through */ case 2: spec->ht.mcs.rx_mask[1] = 0xff; + /* fall through */ case 1: spec->ht.mcs.rx_mask[0] = 0xff; spec->ht.mcs.rx_mask[4] = 0x1; /* MCS32 */ -- 2.7.4
[PATCH 15/20] rt2x00: rt2500pci: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ralink/rt2x00/rt2500pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c index 1ff5434..e8e7bfe 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c @@ -1430,7 +1430,7 @@ static void rt2500pci_txdone(struct rt2x00_dev *rt2x00dev, break; case 2: /* Failure, excessive retries */ __set_bit(TXDONE_EXCESSIVE_RETRY, &txdesc.flags); - /* Don't break, this is a failed frame! */ + /* Fall through - this is a failed frame! */ default: /* Failure */ __set_bit(TXDONE_FAILURE, &txdesc.flags); } -- 2.7.4
[PATCH 13/20] mwifiex: Mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/marvell/mwifiex/ie.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/marvell/mwifiex/ie.c b/drivers/net/wireless/marvell/mwifiex/ie.c index 75cbd60..6845eb5 100644 --- a/drivers/net/wireless/marvell/mwifiex/ie.c +++ b/drivers/net/wireless/marvell/mwifiex/ie.c @@ -363,6 +363,7 @@ static int mwifiex_uap_parse_tail_ies(struct mwifiex_private *priv, (const u8 *)hdr, hdr->len + sizeof(struct ieee_types_header))) break; + /* fall through */ default: memcpy(gen_ie->ie_buffer + ie_len, hdr, hdr->len + sizeof(struct ieee_types_header)); -- 2.7.4
[PATCH 10/20] prism54: isl_38xx: Mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 114944 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/intersil/prism54/isl_38xx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/intersil/prism54/isl_38xx.c b/drivers/net/wireless/intersil/prism54/isl_38xx.c index ce9d4db..b0eb58a 100644 --- a/drivers/net/wireless/intersil/prism54/isl_38xx.c +++ b/drivers/net/wireless/intersil/prism54/isl_38xx.c @@ -235,6 +235,7 @@ isl38xx_in_queue(isl38xx_control_block *cb, int queue) /* send queues */ case ISL38XX_CB_TX_MGMTQ: BUG_ON(delta > ISL38XX_CB_MGMT_QSIZE); + /* fall through */ case ISL38XX_CB_TX_DATA_LQ: case ISL38XX_CB_TX_DATA_HQ: -- 2.7.4
[PATCH 08/20] iwlegacy: common: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 201384 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/intel/iwlegacy/common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c index 6514baf..a2f86cb 100644 --- a/drivers/net/wireless/intel/iwlegacy/common.c +++ b/drivers/net/wireless/intel/iwlegacy/common.c @@ -2695,6 +2695,7 @@ il_set_decrypted_flag(struct il_priv *il, struct ieee80211_hdr *hdr, if ((decrypt_res & RX_RES_STATUS_DECRYPT_TYPE_MSK) == RX_RES_STATUS_BAD_KEY_TTAK) break; + /* fall through */ case RX_RES_STATUS_SEC_TYPE_WEP: if ((decrypt_res & RX_RES_STATUS_DECRYPT_TYPE_MSK) == @@ -2704,6 +2705,7 @@ il_set_decrypted_flag(struct il_priv *il, struct ieee80211_hdr *hdr, D_RX("Packet destroyed\n"); return -1; } + /* fall through */ case RX_RES_STATUS_SEC_TYPE_CCMP: if ((decrypt_res & RX_RES_STATUS_DECRYPT_TYPE_MSK) == RX_RES_STATUS_DECRYPT_OK) { -- 2.7.4
[PATCH 04/20] ath9k: hw: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1056532 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath9k/hw.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c index bb319f2..8581d91 100644 --- a/drivers/net/wireless/ath/ath9k/hw.c +++ b/drivers/net/wireless/ath/ath9k/hw.c @@ -2279,6 +2279,7 @@ void ath9k_hw_beaconinit(struct ath_hw *ah, u32 next_beacon, u32 beacon_period) case NL80211_IFTYPE_ADHOC: REG_SET_BIT(ah, AR_TXCFG, AR_TXCFG_ADHOC_BEACON_ATIM_TX_POLICY); + /* fall through */ case NL80211_IFTYPE_MESH_POINT: case NL80211_IFTYPE_AP: REG_WRITE(ah, AR_NEXT_TBTT_TIMER, next_beacon); -- 2.7.4
[PATCH 03/20] ath9k: ar9002_phy: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1056532 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath9k/ar9002_phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath9k/ar9002_phy.c b/drivers/net/wireless/ath/ath9k/ar9002_phy.c index 7132918..6f32b8d 100644 --- a/drivers/net/wireless/ath/ath9k/ar9002_phy.c +++ b/drivers/net/wireless/ath/ath9k/ar9002_phy.c @@ -119,7 +119,7 @@ static int ar9002_hw_set_channel(struct ath_hw *ah, struct ath9k_channel *chan) aModeRefSel = 2; if (aModeRefSel) break; - /* else: fall through */ + /* fall through */ case 1: default: aModeRefSel = 0; -- 2.7.4
[PATCH 01/20] ath6kl: Mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 201383 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath6kl/main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c index cb59016..5e7ea83 100644 --- a/drivers/net/wireless/ath/ath6kl/main.c +++ b/drivers/net/wireless/ath/ath6kl/main.c @@ -389,6 +389,7 @@ void ath6kl_connect_ap_mode_bss(struct ath6kl_vif *vif, u16 channel) if (!ik->valid || ik->key_type != WAPI_CRYPT) break; /* for WAPI, we need to set the delayed group key, continue: */ + /* fall through */ case WPA_PSK_AUTH: case WPA2_PSK_AUTH: case (WPA_PSK_AUTH | WPA2_PSK_AUTH): -- 2.7.4
Re: [PATCH 0/2] Mark expected switch fall-throughs and fix missing break
On 10/22/18 4:36 PM, Jes Sorensen wrote: > On 10/22/18 7:49 AM, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wimplicit-fallthrough, this patchset aims >> to mark multiple switch cases where we are expecting to fall through. >> >> Also, the second patch in this series fixes a missing break in switch. > > Enabling that sounds like a great way to inflict pain and suffering. > Not really. The -Wimplicit-fallthrough will be enabled until after all the current warnings have been addressed. There are 600 of these issues left. So, hopefully I will complete this task during the next development cycle. Thanks -- Gustavo
[PATCH 2/2] rtl8xxxu: Fix missing break in switch
Add missing break statement in order to prevent the code from falling through to the default case. Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index c6b0686..2bd4305 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -5692,6 +5692,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd, break; case WLAN_CIPHER_SUITE_TKIP: key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC; + break; default: return -EOPNOTSUPP; } -- 2.7.4
[PATCH 1/2] rtl8xxxu: Mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1357355 ("Missing break in switch") Addresses-Coverity-ID: 1357378 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index 56040b1..c6b0686 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct ieee80211_hw *hw) switch (hw->conf.chandef.width) { case NL80211_CHAN_WIDTH_20_NOHT: ht = false; + /* fall through */ case NL80211_CHAN_WIDTH_20: opmode |= BW_OPMODE_20MHZ; rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode); @@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw) switch (hw->conf.chandef.width) { case NL80211_CHAN_WIDTH_20_NOHT: ht = false; + /* fall through */ case NL80211_CHAN_WIDTH_20: rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20; subchannel = 0; @@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv *priv) case 3: priv->ep_tx_low_queue = 1; priv->ep_tx_count++; + /* fall through */ case 2: priv->ep_tx_normal_queue = 1; priv->ep_tx_count++; + /* fall through */ case 1: priv->ep_tx_high_queue = 1; priv->ep_tx_count++; -- 2.7.4
[PATCH 0/2] Mark expected switch fall-throughs and fix missing break
In preparation to enabling -Wimplicit-fallthrough, this patchset aims to mark multiple switch cases where we are expecting to fall through. Also, the second patch in this series fixes a missing break in switch. Thanks Gustavo A. R. Silva (2): rtl8xxxu: Mark expected switch fall-throughs rtl8xxxu: Fix missing break in switch drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 + 1 file changed, 5 insertions(+) -- 2.7.4
Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
On 10/13/18 7:23 PM, Kalle Valo wrote: > > Patch applied to ath-next branch of ath.git, thanks. > > 9d9cdbf3f9ed ath10k: htt_rx: fix signedness bug in > ath10k_update_per_peer_tx_stats > Thank you, Kalle. -- Gustavo
Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
> > I have sent a patch to address this, > https://patchwork.kernel.org/patch/10611943/ > That's great. Thanks for letting me know. :) -- Gustavo
[PATCH] ath10k: remove unnecessary comparison of unsigned integer with < 0
There is no need to compare *ps_state_enable* with < 0 because such variable is of type u8 (8 bits, unsigned), making it impossible to hold a negative value. Fix this by removing such comparison. Addresses-Coverity-ID: 1473921 ("Unsigned compared against 0") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath10k/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index 2c0cb67..15964b3 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -2421,7 +2421,7 @@ static ssize_t ath10k_write_ps_state_enable(struct file *file, if (kstrtou8_from_user(user_buf, count, 0, &ps_state_enable)) return -EINVAL; - if (ps_state_enable > 1 || ps_state_enable < 0) + if (ps_state_enable > 1) return -EINVAL; mutex_lock(&ar->conf_mutex); -- 2.7.4
Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote: > > > On 10/5/18 9:09 PM, Ben Greear wrote: >> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote: >>> Currently, the error handling for the call to function >>> ath10k_get_legacy_rate_idx() doesn't work because >>> *rate_idx* is of type u8 (8 bits, unsigned), which >>> makes it impossible for it to hold a value less >>> than 0. >>> >>> Fix this by changing the type of variable *rate_idx* >>> to s8 (8 bits, signed). >> >> There are more than 127 rates, are you sure this is doing >> what you want? >> > > Based on the following function, rate_idx can only hold values from 0 to 11 > ... and of course -EINVAL too > static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate) > { > static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12, > 18, 24, 36, 48, 54}; > int i; > > for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) { > if (rate == legacy_rates[i]) > return i; > } > > ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate); > return -EINVAL; > } > > Thanks > -- > Gustavo > >> Thanks, >> Ben >> >>> >>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0") >>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the >>> txrate table") >>> Signed-off-by: Gustavo A. R. Silva >>> --- >>> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c >>> b/drivers/net/wireless/ath/ath10k/htt_rx.c >>> index f240525..edd0e74 100644 >>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c >>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c >>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar, >>> struct ath10k_per_peer_tx_stats *peer_stats) >>> { >>> struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv; >>> - u8 rate = 0, rate_idx = 0, sgi; >>> + u8 rate = 0, sgi; >>> + s8 rate_idx = 0; >>> struct rate_info txrate; >>> >>> lockdep_assert_held(&ar->data_lock); >>> >> >>
Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
On 10/5/18 9:09 PM, Ben Greear wrote: > On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote: >> Currently, the error handling for the call to function >> ath10k_get_legacy_rate_idx() doesn't work because >> *rate_idx* is of type u8 (8 bits, unsigned), which >> makes it impossible for it to hold a value less >> than 0. >> >> Fix this by changing the type of variable *rate_idx* >> to s8 (8 bits, signed). > > There are more than 127 rates, are you sure this is doing > what you want? > Based on the following function, rate_idx can only hold values from 0 to 11 static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate) { static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12, 18, 24, 36, 48, 54}; int i; for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) { if (rate == legacy_rates[i]) return i; } ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate); return -EINVAL; } Thanks -- Gustavo > Thanks, > Ben > >> >> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0") >> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate >> table") >> Signed-off-by: Gustavo A. R. Silva >> --- >> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c >> b/drivers/net/wireless/ath/ath10k/htt_rx.c >> index f240525..edd0e74 100644 >> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c >> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c >> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar, >> struct ath10k_per_peer_tx_stats *peer_stats) >> { >> struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv; >> - u8 rate = 0, rate_idx = 0, sgi; >> + u8 rate = 0, sgi; >> + s8 rate_idx = 0; >> struct rate_info txrate; >> >> lockdep_assert_held(&ar->data_lock); >> > >
[PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
Currently, the error handling for the call to function ath10k_get_legacy_rate_idx() doesn't work because *rate_idx* is of type u8 (8 bits, unsigned), which makes it impossible for it to hold a value less than 0. Fix this by changing the type of variable *rate_idx* to s8 (8 bits, signed). Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0") Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c index f240525..edd0e74 100644 --- a/drivers/net/wireless/ath/ath10k/htt_rx.c +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar, struct ath10k_per_peer_tx_stats *peer_stats) { struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv; - u8 rate = 0, rate_idx = 0, sgi; + u8 rate = 0, sgi; + s8 rate_idx = 0; struct rate_info txrate; lockdep_assert_held(&ar->data_lock); -- 2.7.4
[PATCH] NFC: st21nfca: Mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1373889 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/nfc/st21nfca/dep.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/nfc/st21nfca/dep.c b/drivers/nfc/st21nfca/dep.c index 3420c51..6219e92 100644 --- a/drivers/nfc/st21nfca/dep.c +++ b/drivers/nfc/st21nfca/dep.c @@ -620,6 +620,7 @@ static void st21nfca_im_recv_dep_res_cb(void *context, struct sk_buff *skb, switch (ST21NFCA_NFC_DEP_PFB_TYPE(dep_res->pfb)) { case ST21NFCA_NFC_DEP_PFB_ACK_NACK_PDU: pr_err("Received a ACK/NACK PDU\n"); + /* fall through */ case ST21NFCA_NFC_DEP_PFB_I_PDU: info->dep_info.curr_nfc_dep_pni = ST21NFCA_NFC_DEP_PFB_PNI(dep_res->pfb + 1); -- 2.7.4
[PATCH] NFC: pn533: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1230487 ("Missing break in switch") Addresses-Coverity-ID: 1230488 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva --- drivers/nfc/pn533/pn533.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c index a0cc1cc..5961f14 100644 --- a/drivers/nfc/pn533/pn533.c +++ b/drivers/nfc/pn533/pn533.c @@ -2147,6 +2147,7 @@ static int pn533_transceive(struct nfc_dev *nfc_dev, break; } + /* fall through */ default: /* jumbo frame ? */ if (skb->len > PN533_CMD_DATAEXCH_DATA_MAXLEN) { @@ -2273,6 +2274,7 @@ static void pn533_wq_mi_recv(struct work_struct *work) break; } + /* fall through */ default: skb_put_u8(skb, 1); /*TG*/ -- 2.7.4
[PATCH] ssb: chipcommon: fix fall-through annotation
Replace "Fallthough" with a proper "fall through" annotation. This fix is part of the ongoing efforts to enabling -Wimplicit-fallthrough Signed-off-by: Gustavo A. R. Silva --- drivers/ssb/driver_chipcommon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ssb/driver_chipcommon.c b/drivers/ssb/driver_chipcommon.c index 99a4656..3861cb6 100644 --- a/drivers/ssb/driver_chipcommon.c +++ b/drivers/ssb/driver_chipcommon.c @@ -425,7 +425,7 @@ void ssb_chipco_get_clockcontrol(struct ssb_chipcommon *cc, *m = chipco_read32(cc, SSB_CHIPCO_CLOCK_M2); break; } - /* Fallthough */ + /* Fall through */ default: *m = chipco_read32(cc, SSB_CHIPCO_CLOCK_SB); } -- 2.7.4
[PATCH] mwifiex: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/marvell/mwifiex/cfg80211.c | 4 drivers/net/wireless/marvell/mwifiex/scan.c | 1 + 2 files changed, 5 insertions(+) diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c index 54a2297..16a705d 100644 --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c @@ -1158,6 +1158,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy, case NL80211_IFTYPE_UNSPECIFIED: mwifiex_dbg(priv->adapter, INFO, "%s: kept type as IBSS\n", dev->name); + /* fall through */ case NL80211_IFTYPE_ADHOC: /* This shouldn't happen */ return 0; default: @@ -1188,6 +1189,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy, case NL80211_IFTYPE_UNSPECIFIED: mwifiex_dbg(priv->adapter, INFO, "%s: kept type as STA\n", dev->name); + /* fall through */ case NL80211_IFTYPE_STATION:/* This shouldn't happen */ return 0; default: @@ -1210,6 +1212,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy, case NL80211_IFTYPE_UNSPECIFIED: mwifiex_dbg(priv->adapter, INFO, "%s: kept type as AP\n", dev->name); + /* fall through */ case NL80211_IFTYPE_AP: /* This shouldn't happen */ return 0; default: @@ -1249,6 +1252,7 @@ mwifiex_cfg80211_change_virtual_intf(struct wiphy *wiphy, case NL80211_IFTYPE_UNSPECIFIED: mwifiex_dbg(priv->adapter, INFO, "%s: kept type as P2P\n", dev->name); + /* fall through */ case NL80211_IFTYPE_P2P_CLIENT: case NL80211_IFTYPE_P2P_GO: return 0; diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c index d7ce7f7..19df92b 100644 --- a/drivers/net/wireless/marvell/mwifiex/scan.c +++ b/drivers/net/wireless/marvell/mwifiex/scan.c @@ -1308,6 +1308,7 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter, case WLAN_EID_CHANNEL_SWITCH: bss_entry->chan_sw_ie_present = true; + /* fall through */ case WLAN_EID_PWR_CAPABILITY: case WLAN_EID_TPC_REPORT: case WLAN_EID_QUIET: -- 2.7.4
[PATCH] ath9k: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath9k/ar5008_phy.c | 2 ++ drivers/net/wireless/ath/ath9k/ar9002_phy.c | 1 + drivers/net/wireless/ath/ath9k/main.c | 1 + 3 files changed, 4 insertions(+) diff --git a/drivers/net/wireless/ath/ath9k/ar5008_phy.c b/drivers/net/wireless/ath/ath9k/ar5008_phy.c index 7922550..ef2dd68 100644 --- a/drivers/net/wireless/ath/ath9k/ar5008_phy.c +++ b/drivers/net/wireless/ath/ath9k/ar5008_phy.c @@ -583,12 +583,14 @@ static void ar5008_hw_init_chain_masks(struct ath_hw *ah) case 0x5: REG_SET_BIT(ah, AR_PHY_ANALOG_SWAP, AR_PHY_SWAP_ALT_CHAIN); + /* fall through */ case 0x3: if (ah->hw_version.macVersion == AR_SREV_REVISION_5416_10) { REG_WRITE(ah, AR_PHY_RX_CHAINMASK, 0x7); REG_WRITE(ah, AR_PHY_CAL_CHAINMASK, 0x7); break; } + /* else: fall through */ case 0x1: case 0x2: case 0x7: diff --git a/drivers/net/wireless/ath/ath9k/ar9002_phy.c b/drivers/net/wireless/ath/ath9k/ar9002_phy.c index 61a9b85..7132918 100644 --- a/drivers/net/wireless/ath/ath9k/ar9002_phy.c +++ b/drivers/net/wireless/ath/ath9k/ar9002_phy.c @@ -119,6 +119,7 @@ static int ar9002_hw_set_channel(struct ath_hw *ah, struct ath9k_channel *chan) aModeRefSel = 2; if (aModeRefSel) break; + /* else: fall through */ case 1: default: aModeRefSel = 0; diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index a3be8ad..11d84f4 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -1928,6 +1928,7 @@ static int ath9k_ampdu_action(struct ieee80211_hw *hw, case IEEE80211_AMPDU_TX_STOP_FLUSH: case IEEE80211_AMPDU_TX_STOP_FLUSH_CONT: flush = true; + /* fall through */ case IEEE80211_AMPDU_TX_STOP_CONT: ath9k_ps_wakeup(sc); ath_tx_aggr_stop(sc, sta, tid); -- 2.7.4
Re: [PATCH v2] ath6kl: mark expected switch fall-throughs
On 05/25/2018 01:27 PM, Steve deRosier wrote: On Fri, May 25, 2018 at 11:23 AM Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- Changes in v2: - Place code comments on a line of their own. drivers/net/wireless/ath/ath6kl/cfg80211.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c index 2ba8cf3..a16ee5d 100644 --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c @@ -3899,16 +3899,19 @@ int ath6kl_cfg80211_init(struct ath6kl *ar) switch (ar->hw.cap) { case WMI_11AN_CAP: ht = true; + /* fall through */ case WMI_11A_CAP: band_5gig = true; break; case WMI_11GN_CAP: ht = true; + /* fall through */ case WMI_11G_CAP: band_2gig = true; break; case WMI_11AGN_CAP: ht = true; + /* fall through */ case WMI_11AG_CAP: band_2gig = true; band_5gig = true; -- 2.7.4 Gustavo, Thanks for the adjustment. It now looks good to me. Glad to help. :) Reviewed-by: Steve deRosier Thanks -- Gustavo
Re: [PATCH] ath6kl: mark expected switch fall-throughs
On 05/25/2018 01:10 PM, Kalle Valo wrote: Yeah, I was wondering the same. Was there a particular reason for this? Sometimes people use this style for a one-line code block. I can change it to the traditional style. No problem. I would prefer that. So if you can send v2 that would be great. Yep. No problem. I'll send it shortly. Thanks -- Gustavo
[PATCH v2] ath6kl: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- Changes in v2: - Place code comments on a line of their own. drivers/net/wireless/ath/ath6kl/cfg80211.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c index 2ba8cf3..a16ee5d 100644 --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c @@ -3899,16 +3899,19 @@ int ath6kl_cfg80211_init(struct ath6kl *ar) switch (ar->hw.cap) { case WMI_11AN_CAP: ht = true; + /* fall through */ case WMI_11A_CAP: band_5gig = true; break; case WMI_11GN_CAP: ht = true; + /* fall through */ case WMI_11G_CAP: band_2gig = true; break; case WMI_11AGN_CAP: ht = true; + /* fall through */ case WMI_11AG_CAP: band_2gig = true; band_5gig = true; -- 2.7.4
Re: [PATCH] ath6kl: mark expected switch fall-throughs
On 05/25/2018 08:30 AM, Kalle Valo wrote: Sergei Shtylyov writes: On 5/25/2018 2:13 AM, Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath6kl/cfg80211.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c index 2ba8cf3..29e32cd 100644 --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c @@ -3898,17 +3898,17 @@ int ath6kl_cfg80211_init(struct ath6kl *ar) wiphy->max_scan_ie_len = 1000; /* FIX: what is correct limit? */ switch (ar->hw.cap) { case WMI_11AN_CAP: - ht = true; + ht = true; /* fall through */ case WMI_11A_CAP: band_5gig = true; break; case WMI_11GN_CAP: - ht = true; + ht = true; /* fall through */ case WMI_11G_CAP: band_2gig = true; break; case WMI_11AGN_CAP: - ht = true; + ht = true; /* fall through */ case WMI_11AG_CAP: band_2gig = true; band_5gig = true; Hm, typically such comments are done on a line of their own, have never seen this style... Yeah, I was wondering the same. Was there a particular reason for this? Sometimes people use this style for a one-line code block. I can change it to the traditional style. No problem. Thanks -- Gustavo
[PATCH] ath5k: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath5k/pcu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c index f23c851..05140d8 100644 --- a/drivers/net/wireless/ath/ath5k/pcu.c +++ b/drivers/net/wireless/ath/ath5k/pcu.c @@ -670,6 +670,7 @@ ath5k_hw_init_beacon_timers(struct ath5k_hw *ah, u32 next_beacon, u32 interval) break; case NL80211_IFTYPE_ADHOC: AR5K_REG_ENABLE_BITS(ah, AR5K_TXCFG, AR5K_TXCFG_ADHOC_BCN_ATIM); + /* fall through */ default: /* On non-STA modes timer1 is used as next DMA * beacon alert (DBA) timer and timer2 as next -- 2.7.4
[PATCH] ath10k: htt_tx: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in this particular case, I replaced "pass through" with a proper "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath10k/htt_tx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c index 5d8b97a..89157c5 100644 --- a/drivers/net/wireless/ath/ath10k/htt_tx.c +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c @@ -1202,7 +1202,7 @@ static int ath10k_htt_tx_32(struct ath10k_htt *htt, case ATH10K_HW_TXRX_RAW: case ATH10K_HW_TXRX_NATIVE_WIFI: flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT; - /* pass through */ + /* fall through */ case ATH10K_HW_TXRX_ETHERNET: if (ar->hw_params.continuous_frag_desc) { ext_desc_t = htt->frag_desc.vaddr_desc_32; @@ -1404,7 +1404,7 @@ static int ath10k_htt_tx_64(struct ath10k_htt *htt, case ATH10K_HW_TXRX_RAW: case ATH10K_HW_TXRX_NATIVE_WIFI: flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT; - /* pass through */ + /* fall through */ case ATH10K_HW_TXRX_ETHERNET: if (ar->hw_params.continuous_frag_desc) { ext_desc_t = htt->frag_desc.vaddr_desc_64; -- 2.7.4
[PATCH] ath6kl: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath6kl/cfg80211.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c index 2ba8cf3..29e32cd 100644 --- a/drivers/net/wireless/ath/ath6kl/cfg80211.c +++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c @@ -3898,17 +3898,17 @@ int ath6kl_cfg80211_init(struct ath6kl *ar) wiphy->max_scan_ie_len = 1000; /* FIX: what is correct limit? */ switch (ar->hw.cap) { case WMI_11AN_CAP: - ht = true; + ht = true; /* fall through */ case WMI_11A_CAP: band_5gig = true; break; case WMI_11GN_CAP: - ht = true; + ht = true; /* fall through */ case WMI_11G_CAP: band_2gig = true; break; case WMI_11AGN_CAP: - ht = true; + ht = true; /* fall through */ case WMI_11AG_CAP: band_2gig = true; band_5gig = true; -- 2.7.4
Re: [PATCH] rtlwifi: remove duplicate code
Hi Joe, On 05/24/2018 02:24 PM, Joe Perches wrote: On Thu, 2018-05-24 at 13:54 -0500, Gustavo A. R. Silva wrote: Remove and refactor some code in order to avoid having identical code for different branches. True and nice tool and patch submittal thanks. Notice that the logic has been there since 2014. But perhaps the original logic is a defective copy/paste and it should be corrected instead. Can anyone from realtek verify this? I actually used gitk to track down the last changes made to this code and, it doesn't look like a copy/paste issue: commit: c6821613e653aae4f54c75689e229e3f063b7f69 commit: 27a31a60a4de4c1b45e371152bb6e701e1a8cc40 Thanks -- Gustavo Addresses-Coverity-ID: 1426199 ("Identical code for different branches") Signed-off-by: Gustavo A. R. Silva --- .../realtek/rtlwifi/btcoexist/halbtc8723b2ant.c| 23 -- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c index 279fe01..df3facc 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c @@ -2876,25 +2876,10 @@ static void btc8723b2ant_action_hid(struct btc_coexist *btcoexist) btc8723b2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 13); /* sw mechanism */ - if (BTC_WIFI_BW_HT40 == wifi_bw) { - if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) || - (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) { - btc8723b2ant_sw_mechanism(btcoexist, true, true, - false, false); - } else { - btc8723b2ant_sw_mechanism(btcoexist, true, true, - false, false); - } - } else { - if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) || - (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) { - btc8723b2ant_sw_mechanism(btcoexist, false, true, - false, false); - } else { - btc8723b2ant_sw_mechanism(btcoexist, false, true, - false, false); - } - } + if (wifi_bw == BTC_WIFI_BW_HT40) + btc8723b2ant_sw_mechanism(btcoexist, true, true, false, false); + else + btc8723b2ant_sw_mechanism(btcoexist, false, true, false, false); } /* A2DP only / PAN(EDR) only/ A2DP+PAN(HS) */
[PATCH] rtlwifi: remove duplicate code
Remove and refactor some code in order to avoid having identical code for different branches. Notice that the logic has been there since 2014. Addresses-Coverity-ID: 1426199 ("Identical code for different branches") Signed-off-by: Gustavo A. R. Silva --- .../realtek/rtlwifi/btcoexist/halbtc8723b2ant.c| 23 -- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c index 279fe01..df3facc 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c @@ -2876,25 +2876,10 @@ static void btc8723b2ant_action_hid(struct btc_coexist *btcoexist) btc8723b2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 13); /* sw mechanism */ - if (BTC_WIFI_BW_HT40 == wifi_bw) { - if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) || - (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) { - btc8723b2ant_sw_mechanism(btcoexist, true, true, - false, false); - } else { - btc8723b2ant_sw_mechanism(btcoexist, true, true, - false, false); - } - } else { - if ((wifi_rssi_state == BTC_RSSI_STATE_HIGH) || - (wifi_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) { - btc8723b2ant_sw_mechanism(btcoexist, false, true, - false, false); - } else { - btc8723b2ant_sw_mechanism(btcoexist, false, true, - false, false); - } - } + if (wifi_bw == BTC_WIFI_BW_HT40) + btc8723b2ant_sw_mechanism(btcoexist, true, true, false, false); + else + btc8723b2ant_sw_mechanism(btcoexist, false, true, false, false); } /* A2DP only / PAN(EDR) only/ A2DP+PAN(HS) */ -- 2.7.4
[PATCH] staging: wilc1000: fix infinite loop and out-of-bounds access
If i < slot_id is initially true then it will remain true. Also, as i is being decremented it will end up accessing memory out of bounds. Fix this by incrementing *i* instead of decrementing it. Addresses-Coverity-ID: 1468454 ("Infinite loop") Fixes: faa657641081 ("staging: wilc1000: refactor scan() to free kmalloc memory on failure cases") Signed-off-by: Gustavo A. R. Silva --- BTW... at first sight it seems to me that variables slot_id and i should be of type unsigned instead of signed. drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index 3ca0c97..67104e8 100644 --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -608,7 +608,7 @@ wilc_wfi_cfg_alloc_fill_ssid(struct cfg80211_scan_request *request, out_free: - for (i = 0; i < slot_id ; i--) + for (i = 0; i < slot_id; i++) kfree(ntwk->net_info[i].ssid); kfree(ntwk->net_info); -- 2.7.4
Re: [PATCH] rsi: fix a bug in rsi_hal_key_config()
Hi Dan, On 04/27/2018 06:44 AM, Dan Carpenter wrote: On Fri, Apr 27, 2018 at 02:32:20PM +0300, Kalle Valo wrote: Gustavo submitted an identical patch also for this one :) https://patchwork.kernel.org/patch/10365997/ Hey Gustavo, We keep on sending duplicate patches. Most of the static checker people CC kernel-janit...@vger.kernel.org so we can see what's already been sent. Oh, I didn't know that. I'll start CCing that list then. Thanks for letting me know. -- Gustavo
[PATCH] rsi_91x_mac80211: fix structurally dead code
Function rsi_hal_key_config returns before reaching code at line 922 if (status), hence this code is structurally dead. Fix this by storing the value returned by rsi_hal_load_key into _status_ for its further evaluation and use. Addresses-Coverity-ID: 1468409 ("Structurally dead code") Fixes: 4fd6c4762f37 ("rsi: roaming enhancements") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/rsi/rsi_91x_mac80211.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_mac80211.c b/drivers/net/wireless/rsi/rsi_91x_mac80211.c index 766d874..80e7f4f 100644 --- a/drivers/net/wireless/rsi/rsi_91x_mac80211.c +++ b/drivers/net/wireless/rsi/rsi_91x_mac80211.c @@ -911,14 +911,14 @@ static int rsi_hal_key_config(struct ieee80211_hw *hw, } } - return rsi_hal_load_key(adapter->priv, - key->key, - key->keylen, - key_type, - key->keyidx, - key->cipher, - sta_id, - vif); + status = rsi_hal_load_key(adapter->priv, + key->key, + key->keylen, + key_type, + key->keyidx, + key->cipher, + sta_id, + vif); if (status) return status; -- 2.7.4
[PATCH] rsi_91x_usb: fix uninitialized variable
There is a potential execution path in which variable ret is returned without being properly initialized previously. Fix this by storing the value returned by function rsi_usb_master_reg_write into _ret_. Addresses-Coverity-ID: 1468407 ("Uninitialized scalar variable") Fixes: 16d3bb7b2f37 ("rsi: disable fw watchdog timer during reset") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/rsi/rsi_91x_usb.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c index b065438..6ce6b75 100644 --- a/drivers/net/wireless/rsi/rsi_91x_usb.c +++ b/drivers/net/wireless/rsi/rsi_91x_usb.c @@ -687,9 +687,10 @@ static int rsi_reset_card(struct rsi_hw *adapter) */ msleep(100); - if (rsi_usb_master_reg_write(adapter, SWBL_REGOUT, -RSI_FW_WDT_DISABLE_REQ, -RSI_COMMON_REG_SIZE) < 0) { + ret = rsi_usb_master_reg_write(adapter, SWBL_REGOUT, + RSI_FW_WDT_DISABLE_REQ, + RSI_COMMON_REG_SIZE); + if (ret < 0) { rsi_dbg(ERR_ZONE, "Disabling firmware watchdog timer failed\n"); goto fail; } -- 2.7.4
Re: [PATCH] qtnfmac: pearl: pcie: fix memory leak in qtnf_fw_work_handler
Hi Sergey, On 04/05/2018 11:31 AM, Sergey Matyukevich wrote: Hello Gustavo, In case memory resources for fw were succesfully allocated, release them before jumping to fw_load_fail. Addresses-Coverity-ID: 1466092 ("Resource leak") Fixes: c3b2f7ca4186 ("qtnfmac: implement asynchronous firmware loading") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 4 1 file changed, 4 insertions(+) Thanks for the patch! Glad to help. :) Reviewed-by: Sergey Matyukevich Thanks -- Gustavo
[PATCH] qtnfmac: pearl: pcie: fix memory leak in qtnf_fw_work_handler
In case memory resources for fw were succesfully allocated, release them before jumping to fw_load_fail. Addresses-Coverity-ID: 1466092 ("Resource leak") Fixes: c3b2f7ca4186 ("qtnfmac: implement asynchronous firmware loading") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c index f117904..6c1e139 100644 --- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c +++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c @@ -1185,6 +1185,10 @@ static void qtnf_fw_work_handler(struct work_struct *work) if (qtnf_poll_state(&priv->bda->bda_ep_state, QTN_EP_FW_LOADRDY, QTN_FW_DL_TIMEOUT_MS)) { pr_err("card is not ready\n"); + + if (!flashboot) + release_firmware(fw); + goto fw_load_fail; } -- 2.7.4
[rtlwifi-btcoex] Suspicious code in halbtc8821a1ant driver
Hi all, While doing some static analysis I came across the following piece of code at drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c:1581: 1581 static void btc8821a1ant_act_bt_sco_hid_only_busy(struct btc_coexist *btcoexist, 1582 u8 wifi_status) 1583 { 1584 /* tdma and coex table */ 1585 btc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 5); 1586 1587 if (BT_8821A_1ANT_WIFI_STATUS_NON_CONNECTED_ASSO_AUTH_SCAN == 1588 wifi_status) 1589 btc8821a1ant_coex_table_with_type(btcoexist, NORMAL_EXEC, 1); 1590 else 1591 btc8821a1ant_coex_table_with_type(btcoexist, NORMAL_EXEC, 1); 1592 } The issue here is that the code for both branches of the if-else statement is identical. The if-else was introduced a year ago in this commit c6821613e653 I wonder if an argument should be changed in any of the calls to btc8821a1ant_coex_table_with_type? What do you think? Thanks -- Gustavo
[PATCH] brcm80211: brcmsmac: phy_lcn: remove duplicate code
Remove and refactor some code in order to avoid having identical code for different branches. Notice that this piece of code hasn't been modified since 2011. Addresses-Coverity-ID: 1226756 ("Identical code for different branches") Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c index 93d4cde..9d830d2 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c @@ -3388,13 +3388,8 @@ void wlc_lcnphy_deaf_mode(struct brcms_phy *pi, bool mode) u8 phybw40; phybw40 = CHSPEC_IS40(pi->radio_chanspec); - if (LCNREV_LT(pi->pubpi.phy_rev, 2)) { - mod_phy_reg(pi, 0x4b0, (0x1 << 5), (mode) << 5); - mod_phy_reg(pi, 0x4b1, (0x1 << 9), 0 << 9); - } else { - mod_phy_reg(pi, 0x4b0, (0x1 << 5), (mode) << 5); - mod_phy_reg(pi, 0x4b1, (0x1 << 9), 0 << 9); - } + mod_phy_reg(pi, 0x4b0, (0x1 << 5), (mode) << 5); + mod_phy_reg(pi, 0x4b1, (0x1 << 9), 0 << 9); if (phybw40 == 0) { mod_phy_reg((pi), 0x410, -- 2.7.4
[PATCH] mt7601u: phy: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/mediatek/mt7601u/phy.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/mediatek/mt7601u/phy.c b/drivers/net/wireless/mediatek/mt7601u/phy.c index ca09a5d..9a90f1f 100644 --- a/drivers/net/wireless/mediatek/mt7601u/phy.c +++ b/drivers/net/wireless/mediatek/mt7601u/phy.c @@ -795,6 +795,7 @@ mt7601u_phy_rf_pa_mode_val(struct mt7601u_dev *dev, int phy_mode, int tx_rate) switch (phy_mode) { case MT_PHY_TYPE_OFDM: tx_rate += 4; + /* fall through */ case MT_PHY_TYPE_CCK: reg = dev->rf_pa_mode[0]; break; -- 2.7.4
[PATCH] ath9k: dfs: remove accidental use of stack VLA
In preparation to enabling -Wvla, remove accidental use of stack VLA. This avoids an accidental stack VLA (since the compiler thinks the value of FFT_NUM_SAMPLES can change, even when marked "const"). This just replaces it with a #define. Also, fixed as part of the directive to remove all VLAs from the kernel: https://lkml.org/lkml/2018/3/7/621 Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath9k/dfs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/dfs.c b/drivers/net/wireless/ath/ath9k/dfs.c index 6fee9a4..c8844f5 100644 --- a/drivers/net/wireless/ath/ath9k/dfs.c +++ b/drivers/net/wireless/ath/ath9k/dfs.c @@ -41,7 +41,7 @@ static const int BIN_DELTA_MAX= 10; /* we need at least 3 deltas / 4 samples for a reliable chirp detection */ #define NUM_DIFFS 3 -static const int FFT_NUM_SAMPLES = (NUM_DIFFS + 1); +#define FFT_NUM_SAMPLES(NUM_DIFFS + 1) /* Threshold for difference of delta peaks */ static const int MAX_DIFF = 2; @@ -114,7 +114,7 @@ static bool ath9k_check_chirping(struct ath_softc *sc, u8 *data, ath_dbg(common, DFS, "HT40: datalen=%d, num_fft_packets=%d\n", datalen, num_fft_packets); - if (num_fft_packets < (FFT_NUM_SAMPLES)) { + if (num_fft_packets < FFT_NUM_SAMPLES) { ath_dbg(common, DFS, "not enough packets for chirp\n"); return false; } @@ -136,7 +136,7 @@ static bool ath9k_check_chirping(struct ath_softc *sc, u8 *data, return false; ath_dbg(common, DFS, "HT20: datalen=%d, num_fft_packets=%d\n", datalen, num_fft_packets); - if (num_fft_packets < (FFT_NUM_SAMPLES)) { + if (num_fft_packets < FFT_NUM_SAMPLES) { ath_dbg(common, DFS, "not enough packets for chirp\n"); return false; } -- 2.7.4
Re: [PATCH] mac80211: aes-cmac: remove VLA usage
On 03/21/2018 08:58 AM, Johannes Berg wrote: On Wed, 2018-03-21 at 08:57 -0500, Gustavo A. R. Silva wrote: SHA_DESC_ON_STACK is currently being used in multiple places. But, yeah, I think we can define multiple macros of the same kind and adjust to the characteristics of each the component. How big do you think tfm can get? I have no idea, I guess you'll have to take that with Herbert. johannes I see. I'll contact him then. Thanks for the feedback. -- Gustavo
Re: [PATCH] mac80211: aes-cmac: remove VLA usage
On 03/21/2018 08:48 AM, Johannes Berg wrote: On Wed, 2018-03-21 at 08:42 -0500, Gustavo A. R. Silva wrote: In preparation to enabling -Wvla, remove VLAs and replace them with dynamic memory allocation instead. The use of stack Variable Length Arrays needs to be avoided, as they can be a vector for stack exhaustion, which can be both a runtime bug or a security flaw. Also, in general, as code evolves it is easy to lose track of how big a VLA can get. Thus, we can end up having runtime failures that are hard to debug. Also, fixed as part of the directive to remove all VLAs from the kernel: https://lkml.org/lkml/2018/3/7/621 Signed-off-by: Gustavo A. R. Silva --- net/mac80211/aes_cmac.c | 36 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c index 2fb6558..c9444bf 100644 --- a/net/mac80211/aes_cmac.c +++ b/net/mac80211/aes_cmac.c @@ -27,30 +27,42 @@ static const u8 zero[CMAC_TLEN_256]; void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad, const u8 *data, size_t data_len, u8 *mic) { - SHASH_DESC_ON_STACK(desc, tfm); + struct shash_desc *shash; u8 out[AES_BLOCK_SIZE]; - desc->tfm = tfm; + shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(tfm), + GFP_KERNEL); + if (!shash) + return; Honestly, this seems like a really bad idea - you're now hitting kmalloc for every TX/RX frame here. SHA_DESC_ON_STACK() should just be fixed to not need a VLA, but take some sort of maximum, I guess? SHA_DESC_ON_STACK is currently being used in multiple places. But, yeah, I think we can define multiple macros of the same kind and adjust to the characteristics of each the component. How big do you think tfm can get? Thanks -- Gustavo
[PATCH] mac80211: aes-cmac: remove VLA usage
In preparation to enabling -Wvla, remove VLAs and replace them with dynamic memory allocation instead. The use of stack Variable Length Arrays needs to be avoided, as they can be a vector for stack exhaustion, which can be both a runtime bug or a security flaw. Also, in general, as code evolves it is easy to lose track of how big a VLA can get. Thus, we can end up having runtime failures that are hard to debug. Also, fixed as part of the directive to remove all VLAs from the kernel: https://lkml.org/lkml/2018/3/7/621 Signed-off-by: Gustavo A. R. Silva --- net/mac80211/aes_cmac.c | 36 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c index 2fb6558..c9444bf 100644 --- a/net/mac80211/aes_cmac.c +++ b/net/mac80211/aes_cmac.c @@ -27,30 +27,42 @@ static const u8 zero[CMAC_TLEN_256]; void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad, const u8 *data, size_t data_len, u8 *mic) { - SHASH_DESC_ON_STACK(desc, tfm); + struct shash_desc *shash; u8 out[AES_BLOCK_SIZE]; - desc->tfm = tfm; + shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(tfm), + GFP_KERNEL); + if (!shash) + return; - crypto_shash_init(desc); - crypto_shash_update(desc, aad, AAD_LEN); - crypto_shash_update(desc, data, data_len - CMAC_TLEN); - crypto_shash_finup(desc, zero, CMAC_TLEN, out); + shash->tfm = tfm; + + crypto_shash_init(shash); + crypto_shash_update(shash, aad, AAD_LEN); + crypto_shash_update(shash, data, data_len - CMAC_TLEN); + crypto_shash_finup(shash, zero, CMAC_TLEN, out); memcpy(mic, out, CMAC_TLEN); + kfree(shash); } void ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad, const u8 *data, size_t data_len, u8 *mic) { - SHASH_DESC_ON_STACK(desc, tfm); + struct shash_desc *shash; + + shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(tfm), + GFP_KERNEL); + if (!shash) + return; - desc->tfm = tfm; + shash->tfm = tfm; - crypto_shash_init(desc); - crypto_shash_update(desc, aad, AAD_LEN); - crypto_shash_update(desc, data, data_len - CMAC_TLEN_256); - crypto_shash_finup(desc, zero, CMAC_TLEN_256, mic); + crypto_shash_init(shash); + crypto_shash_update(shash, aad, AAD_LEN); + crypto_shash_update(shash, data, data_len - CMAC_TLEN_256); + crypto_shash_finup(shash, zero, CMAC_TLEN_256, mic); + kfree(shash); } struct crypto_shash *ieee80211_aes_cmac_key_setup(const u8 key[], -- 2.7.4
Re: [PATCH] drivers: net: wireless: ath: ath9: dfs: remove VLA usage
On 03/10/2018 05:12 PM, Kees Cook wrote: On Sat, Mar 10, 2018 at 3:06 PM, Arend van Spriel wrote: On 3/9/2018 1:30 PM, Andreas Christoforou wrote: The kernel would like to have all stack VLA usage removed. I think there was a remark made earlier to give more explanation here. It should explain why we want "VLA on stack" removed. Signed-off-by: Andreas Christoforou --- drivers/net/wireless/ath/ath9k/dfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/dfs.c b/drivers/net/wireless/ath/ath9k/dfs.c index 6fee9a4..cfb0f84 100644 --- a/drivers/net/wireless/ath/ath9k/dfs.c +++ b/drivers/net/wireless/ath/ath9k/dfs.c @@ -41,7 +41,6 @@ static const int BIN_DELTA_MAX= 10; /* we need at least 3 deltas / 4 samples for a reliable chirp detection */ #define NUM_DIFFS 3 -static const int FFT_NUM_SAMPLES = (NUM_DIFFS + 1); What about this instead? #define FFT_NUM_SAMPLES (NUM_DIFFS + 1) /* Threshold for difference of delta peaks */ static const int MAX_DIFF = 2; @@ -101,7 +100,7 @@ static bool ath9k_check_chirping(struct ath_softc *sc, u8 *data, int datalen, bool is_ctl, bool is_ext) { int i; - int max_bin[FFT_NUM_SAMPLES]; + int max_bin[NUM_DIFFS + 1]; Just wondering. Is this actually a VLA. FFT_NUM_SAMPLES was static const so not really going to show a lot of variation. This array will always have the same size on the stack. The problem is that it's not a "constant expression", so the compiler frontend still yells about it under -Wvla. I would characterize this mainly as a fix for "accidental VLA" or "misdetected VLA" or something like that. AIUI, there really isn't a functional change here. -Kees -- Gustavo
[PATCH] ssb: return boolean instead of integer in ssb_dma_translation_special_bit
Return statements in functions returning bool should use true/false instead of 1/0. This issue was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva --- drivers/ssb/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c index 65420a9..fdb89b6 100644 --- a/drivers/ssb/main.c +++ b/drivers/ssb/main.c @@ -1116,7 +1116,7 @@ static bool ssb_dma_translation_special_bit(struct ssb_device *dev) chip_id == 43231 || chip_id == 43222); } - return 0; + return false; } u32 ssb_dma_translation(struct ssb_device *dev) -- 2.7.4
[PATCH] rsi: rsi_91x_ps: remove redundant code in str_psstate
"INVALID_STATE" is already being returned in the default case and this code cannot be reached. Addresses-Coverity-ID: 1398384 Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/rsi/rsi_91x_ps.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/wireless/rsi/rsi_91x_ps.c b/drivers/net/wireless/rsi/rsi_91x_ps.c index 523f532..01472fa 100644 --- a/drivers/net/wireless/rsi/rsi_91x_ps.c +++ b/drivers/net/wireless/rsi/rsi_91x_ps.c @@ -36,7 +36,6 @@ char *str_psstate(enum ps_state state) default: return "INVALID_STATE"; } - return "INVALID_STATE"; } static inline void rsi_modify_ps_state(struct rsi_hw *adapter, -- 2.7.4
[PATCH] ath9k: dfs: use swap macro in ath9k_check_chirping
Make use of the swap macro and remove unnecessary variable temp. This makes the code easier to read and maintain. This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath9k/dfs.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/dfs.c b/drivers/net/wireless/ath/ath9k/dfs.c index 40a397f..6fee9a4 100644 --- a/drivers/net/wireless/ath/ath9k/dfs.c +++ b/drivers/net/wireless/ath/ath9k/dfs.c @@ -123,11 +123,9 @@ static bool ath9k_check_chirping(struct ath_softc *sc, u8 *data, fft = (struct ath9k_dfs_fft_40 *) (data + 2); ath_dbg(common, DFS, "fixing datalen by 2\n"); } - if (IS_CHAN_HT40MINUS(ah->curchan)) { - int temp = is_ctl; - is_ctl = is_ext; - is_ext = temp; - } + if (IS_CHAN_HT40MINUS(ah->curchan)) + swap(is_ctl, is_ext); + for (i = 0; i < FFT_NUM_SAMPLES; i++) max_bin[i] = ath9k_get_max_index_ht40(fft + i, is_ctl, is_ext); -- 2.7.4
[PATCH] net: wireless: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Signed-off-by: Gustavo A. R. Silva --- This code was tested by compilation only (GCC 7.2.0 was used). Please, verify if the actual intention of the code is to fall through. net/wireless/chan.c| 2 ++ net/wireless/nl80211.c | 10 ++ net/wireless/scan.c| 3 ++- net/wireless/wext-compat.c | 2 ++ 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/net/wireless/chan.c b/net/wireless/chan.c index eb82427..6072613 100644 --- a/net/wireless/chan.c +++ b/net/wireless/chan.c @@ -741,6 +741,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, case NL80211_CHAN_WIDTH_20: if (!ht_cap->ht_supported) return false; + /* fall through */ case NL80211_CHAN_WIDTH_20_NOHT: prohibited_flags |= IEEE80211_CHAN_NO_20MHZ; width = 20; @@ -763,6 +764,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy, cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK; if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ) return false; + /* fall through */ case NL80211_CHAN_WIDTH_80: if (!vht_cap->vht_supported) return false; diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index fce2cbe..a8bbb6c 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -1509,6 +1509,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 1: if (nla_put(msg, NL80211_ATTR_CIPHER_SUITES, sizeof(u32) * rdev->wiphy.n_cipher_suites, @@ -1555,6 +1556,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 2: if (nl80211_put_iftypes(msg, NL80211_ATTR_SUPPORTED_IFTYPES, rdev->wiphy.interface_modes)) @@ -1562,6 +1564,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 3: nl_bands = nla_nest_start(msg, NL80211_ATTR_WIPHY_BANDS); if (!nl_bands) @@ -1587,6 +1590,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->chan_start++; if (state->split) break; + /* fall through */ default: /* add frequencies */ nl_freqs = nla_nest_start( @@ -1640,6 +1644,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 4: nl_cmds = nla_nest_start(msg, NL80211_ATTR_SUPPORTED_COMMANDS); if (!nl_cmds) @@ -1666,6 +1671,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 5: if (rdev->ops->remain_on_channel && (rdev->wiphy.flags & WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL) && @@ -1683,6 +1689,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 6: #ifdef CONFIG_PM if (nl80211_send_wowlan(msg, rdev, state->split)) @@ -1693,6 +1700,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, #else state->split_start++; #endif + /* fall through */ case 7: if (nl80211_put_iftypes(msg, NL80211_ATTR_SOFTWARE_IFTYPES, rdev->wiphy.software_iftypes)) @@ -1705,6 +1713,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, state->split_start++; if (state->split) break; + /* fall through */ case 8: if ((rdev->wiphy.flags & WIPHY_FLAG_HAVE_AP_SME) &&
[PATCH] net: mac80211: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Notice that in some cases I replaced "fall through on else" and "otherwise fall through" comments with just a "fall through" comment, which is what GCC is expecting to find. Signed-off-by: Gustavo A. R. Silva --- This code was tested by compilation only (GCC 7.2.0 was used). Please, verify that the actual intention of the code is to fall through. net/mac80211/cfg.c| 3 +++ net/mac80211/ht.c | 1 + net/mac80211/iface.c | 2 +- net/mac80211/mesh.c | 2 ++ net/mac80211/mesh_hwmp.c | 1 + net/mac80211/mesh_plink.c | 2 +- net/mac80211/mlme.c | 1 + net/mac80211/offchannel.c | 4 ++-- net/mac80211/tdls.c | 1 + net/mac80211/wme.c| 1 + 10 files changed, 14 insertions(+), 4 deletions(-) diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index a354f19..9bd8bef 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -573,10 +573,12 @@ static int ieee80211_get_key(struct wiphy *wiphy, struct net_device *dev, case WLAN_CIPHER_SUITE_BIP_CMAC_256: BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) != offsetof(typeof(kseq), aes_cmac)); + /* fall through */ case WLAN_CIPHER_SUITE_BIP_GMAC_128: case WLAN_CIPHER_SUITE_BIP_GMAC_256: BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) != offsetof(typeof(kseq), aes_gmac)); + /* fall through */ case WLAN_CIPHER_SUITE_GCMP: case WLAN_CIPHER_SUITE_GCMP_256: BUILD_BUG_ON(offsetof(typeof(kseq), ccmp) != @@ -2205,6 +2207,7 @@ static int ieee80211_scan(struct wiphy *wiphy, * for now fall through to allow scanning only when * beaconing hasn't been configured yet */ + /* fall through */ case NL80211_IFTYPE_AP: /* * If the scan has been forced (and the driver supports diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c index 41f5e48..e55dabf 100644 --- a/net/mac80211/ht.c +++ b/net/mac80211/ht.c @@ -491,6 +491,7 @@ int ieee80211_send_smps_action(struct ieee80211_sub_if_data *sdata, case IEEE80211_SMPS_AUTOMATIC: case IEEE80211_SMPS_NUM_MODES: WARN_ON(1); + /* fall through */ case IEEE80211_SMPS_OFF: action_frame->u.action.u.ht_smps.smps_control = WLAN_HT_SMPS_CONTROL_DISABLED; diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index 13b16f9..435e735 100644 --- a/net/mac80211/iface.c +++ b/net/mac80211/iface.c @@ -1633,7 +1633,7 @@ static void ieee80211_assign_perm_addr(struct ieee80211_local *local, goto out_unlock; } } - /* otherwise fall through */ + /* fall through */ default: /* assign a new address if possible -- try n_addresses first */ for (i = 0; i < local->hw.wiphy->n_addresses; i++) { diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c index 7a76c4a..d29a545 100644 --- a/net/mac80211/mesh.c +++ b/net/mac80211/mesh.c @@ -988,8 +988,10 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata, switch (sdata->vif.bss_conf.chandef.width) { case NL80211_CHAN_WIDTH_20_NOHT: sta_flags |= IEEE80211_STA_DISABLE_HT; + /* fall through */ case NL80211_CHAN_WIDTH_20: sta_flags |= IEEE80211_STA_DISABLE_40MHZ; + /* fall through */ case NL80211_CHAN_WIDTH_40: sta_flags |= IEEE80211_STA_DISABLE_VHT; break; diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c index 146ec6c..0e75abf 100644 --- a/net/mac80211/mesh_hwmp.c +++ b/net/mac80211/mesh_hwmp.c @@ -1247,6 +1247,7 @@ void mesh_path_tx_root_frame(struct ieee80211_sub_if_data *sdata) break; case IEEE80211_PROACTIVE_PREQ_WITH_PREP: flags |= IEEE80211_PREQ_PROACTIVE_PREP_FLAG; + /* fall through */ case IEEE80211_PROACTIVE_PREQ_NO_PREP: interval = ifmsh->mshcfg.dot11MeshHWMPactivePathToRootTimeout; target_flags |= IEEE80211_PREQ_TO_FLAG | diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c index e2d00cc..0f6c9ca 100644 --- a/net/mac80211/mesh_plink.c +++ b/net/mac80211/mesh_plink.c @@ -672,7 +672,7 @@ void mesh_plink_timer(struct timer_list *t) break; } reason = WLAN_REASON_MESH_MAX_RETRIES; - /* fall through on else */ + /* fall through */ case NL80211_PLINK_CNF_RCVD: /* confirm timer */ if (!reason) diff --git a/net/mac80211/mlme.c b/net/
Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs
Hi Jes, Quoting Jes Sorensen : On 10/11/2017 04:41 AM, Kalle Valo wrote: Jes Sorensen writes: On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote: In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. While this isn't harmful, to me this looks like pointless patch churn for zero gain and it's just ugly. In general I find it useful to mark fall through cases. And it's just a comment with two words, so they cannot hurt your eyes that much. I don't see them being harmful in the code, but I don't see them of much use either. If it happened as part of natural code development, fine. My objection is to people running around doing this systematically causing patch churn for little to zero gain. Jes I understand that you think this is of zero gain for you, but as Florian Fainelli pointed out: "That is the canonical way to tell static analyzers and compilers that fall throughs are wanted and not accidental mistakes in the code. For people that deal with these kinds of errors, it's quite helpful, unless you suggest disabling that particular GCC warning specific for that file/directory?" this is very helpful for people working on fixing issues reported by static analyzers. It saves a huge amount of time when dealing with False Positives. Also, there are cases when an apparently intentional fall-through turns out to be an actual missing break or continue. So there is an ongoing effort to detect such cases and avoid them to show up in the future by at least warning people about a potential issue in their code. And this is helpful for everybody. Thanks -- Gustavo A. R. Silva
[PATCH] rtl8xxxu: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Cc: Jes Sorensen Cc: Kalle Valo Cc: linux-wireless@vger.kernel.org Cc: net...@vger.kernel.org Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index 7806a4d..e66be05 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct ieee80211_hw *hw) switch (hw->conf.chandef.width) { case NL80211_CHAN_WIDTH_20_NOHT: ht = false; + /* fall through */ case NL80211_CHAN_WIDTH_20: opmode |= BW_OPMODE_20MHZ; rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode); @@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw) switch (hw->conf.chandef.width) { case NL80211_CHAN_WIDTH_20_NOHT: ht = false; + /* fall through */ case NL80211_CHAN_WIDTH_20: rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20; subchannel = 0; @@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv *priv) case 3: priv->ep_tx_low_queue = 1; priv->ep_tx_count++; + /* fall through */ case 2: priv->ep_tx_normal_queue = 1; priv->ep_tx_count++; + /* fall through */ case 1: priv->ep_tx_high_queue = 1; priv->ep_tx_count++; @@ -5691,6 +5695,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd, break; case WLAN_CIPHER_SUITE_TKIP: key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC; + /* fall through */ default: return -EOPNOTSUPP; } -- 2.7.4
Re: [PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches
Hi Larry, On 08/30/2017 11:48 PM, Larry Finger wrote: On 08/30/2017 08:42 AM, Gustavo A. R. Silva wrote: Refactor code in order to avoid identical code for different branches. This issue was detected with the help of Coccinelle. Addresses-Coverity-ID: 1226788 Signed-off-by: Gustavo A. R. Silva --- This issue was reported by Coverity and it was tested by compilation only. I'm suspicious this may be a copy/paste error. Please, verify. .../net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) This change is not correct. When bt_link_info->sco_exist is true, the call should be halbtc8723b1ant_limited_rx(btcoexist, NORMAL_EXEC, true, false, 0x5); NACK I will push the correct patch. Great. Good to know. Thanks -- Gustavo A. R. Silva
[PATCH] rtlwifi: rtl8723be: fix duplicated code for different branches
Refactor code in order to avoid identical code for different branches. Addresses-Coverity-ID: 1248728 Signed-off-by: Gustavo A. R. Silva --- This issue was reported by Coverity and it was tested by compilation only. Please, verify if this is not a copy/paste error. Also, notice this code has been there since 2014. drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c index 131c0d1..15c117e 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c @@ -883,12 +883,8 @@ static void rtl8723be_dm_txpower_tracking_callback_thermalmeter( if ((rtldm->power_index_offset[RF90_PATH_A] != 0) && (rtldm->txpower_track_control)) { rtldm->done_txpower = true; - if (thermalvalue > rtlefuse->eeprom_thermalmeter) - rtl8723be_dm_tx_power_track_set_power(hw, BBSWING, 0, -index_for_channel); - else - rtl8723be_dm_tx_power_track_set_power(hw, BBSWING, 0, -index_for_channel); + rtl8723be_dm_tx_power_track_set_power(hw, BBSWING, 0, + index_for_channel); rtldm->swing_idx_cck_base = rtldm->swing_idx_cck; rtldm->swing_idx_ofdm_base[RF90_PATH_A] = -- 2.5.0
Re: [PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches
Hi Larry, On 08/30/2017 11:37 AM, Larry Finger wrote: On 08/30/2017 08:42 AM, Gustavo A. R. Silva wrote: Refactor code in order to avoid identical code for different branches. This issue was detected with the help of Coccinelle. Addresses-Coverity-ID: 1226788 Signed-off-by: Gustavo A. R. Silva --- This issue was reported by Coverity and it was tested by compilation only. I'm suspicious this may be a copy/paste error. Please, verify. I have referred this change to the engineers at Realtek. For the moment, please hold this patch. Thanks for reporting the condition. Glad to help. :) -- Gustavo A. R. Silva
[PATCH] rtlwifi: refactor code in halbtcoutsrc module
Function halbtc_get_wifi_rssi always returns rtlpriv->dm.undec_sm_pwdb. So this function can be removed and the value of rtlpriv->dm.undec_sm_pwdb assigned to *s32_tmp directly. This issue was first reported by Coverity as "identical code for different branches" in function halbtc_get_wifi_rssi. Addresses-Coverity-ID: 1226793 Signed-off-by: Gustavo A. R. Silva --- This code was reported by Coverity and it was tested by compilation only. Chances are this may be a copy/paste error in function halbtc_get_wifi_rssi. Please, verify. Also, notice this code has been there since 2014. .../net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c index c1eacd8..2a47b97 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c @@ -373,17 +373,6 @@ u32 halbtc_get_wifi_link_status(struct btc_coexist *btcoexist) return ret_val; } -static s32 halbtc_get_wifi_rssi(struct rtl_priv *rtlpriv) -{ - int undec_sm_pwdb = 0; - - if (rtlpriv->mac80211.link_state >= MAC80211_LINKED) - undec_sm_pwdb = rtlpriv->dm.undec_sm_pwdb; - else /* associated entry pwdb */ - undec_sm_pwdb = rtlpriv->dm.undec_sm_pwdb; - return undec_sm_pwdb; -} - static bool halbtc_get(void *void_btcoexist, u8 get_type, void *out_buf) { struct btc_coexist *btcoexist = (struct btc_coexist *)void_btcoexist; @@ -479,7 +468,7 @@ static bool halbtc_get(void *void_btcoexist, u8 get_type, void *out_buf) *bool_tmp = false; break; case BTC_GET_S4_WIFI_RSSI: - *s32_tmp = halbtc_get_wifi_rssi(rtlpriv); + *s32_tmp = rtlpriv->dm.undec_sm_pwdb; break; case BTC_GET_S4_HS_RSSI: *s32_tmp = 0; -- 2.5.0
[PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches
Refactor code in order to avoid identical code for different branches. This issue was detected with the help of Coccinelle. Addresses-Coverity-ID: 1226788 Signed-off-by: Gustavo A. R. Silva --- This issue was reported by Coverity and it was tested by compilation only. I'm suspicious this may be a copy/paste error. Please, verify. .../net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c index c044252..960ce80f 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c @@ -2260,14 +2260,8 @@ static void halbtc8723b1ant_run_coexist_mechanism(struct btc_coexist *btcoexist) if (iot_peer != BTC_IOT_PEER_CISCO && iot_peer != BTC_IOT_PEER_BROADCOM) { - if (bt_link_info->sco_exist) - halbtc8723b1ant_limited_rx(btcoexist, - NORMAL_EXEC, false, - false, 0x5); - else - halbtc8723b1ant_limited_rx(btcoexist, - NORMAL_EXEC, false, - false, 0x5); + halbtc8723b1ant_limited_rx(btcoexist, NORMAL_EXEC, + false, false, 0x5); } else { if (bt_link_info->sco_exist) { halbtc8723b1ant_limited_rx(btcoexist, -- 2.5.0
[PATCH] rtlwifi: btcoex: 23b 1ant: fix duplicated code for different branches
Refactor code in order to avoid identical code for different branches. This issue was detected with the help of Coccinelle. Addresses-Coverity-ID: 1415177 Signed-off-by: Gustavo A. R. Silva --- This issue was reported by Coverity and it was tested by compilation only. .../net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c index 03998d2..c044252 100644 --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c @@ -600,14 +600,8 @@ static void halbtc8723b1ant_coex_table_with_type(struct btc_coexist *btcoexist, 0xff, 0x3); break; case 5: - if ((coex_sta->cck_ever_lock) && (coex_sta->scan_ap_num <= 5)) - halbtc8723b1ant_coex_table(btcoexist, force_exec, - 0x5a5a5a5a, 0x5aaa5a5a, - 0xff, 0x3); - else - halbtc8723b1ant_coex_table(btcoexist, force_exec, - 0x5a5a5a5a, 0x5aaa5a5a, - 0xff, 0x3); + halbtc8723b1ant_coex_table(btcoexist, force_exec, 0x5a5a5a5a, + 0x5aaa5a5a, 0xff, 0x3); break; case 6: halbtc8723b1ant_coex_table(btcoexist, force_exec, 0x, -- 2.5.0
[PATCH] rtlwifi: remove useless code
Remove useless local variables last_read_point and last_txw_point and the code related. Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c index 55f238a..c58393e 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/trx.c @@ -478,7 +478,6 @@ u16 rtl92ee_rx_desc_buff_remained_cnt(struct ieee80211_hw *hw, u8 queue_index) struct rtl_priv *rtlpriv = rtl_priv(hw); u16 read_point = 0, write_point = 0, remind_cnt = 0; u32 tmp_4byte = 0; - static u16 last_read_point; static bool start_rx; tmp_4byte = rtl_read_dword(rtlpriv, REG_RXQ_TXBD_IDX); @@ -506,7 +505,6 @@ u16 rtl92ee_rx_desc_buff_remained_cnt(struct ieee80211_hw *hw, u8 queue_index) rtlpci->rx_ring[queue_index].next_rx_rp = write_point; - last_read_point = read_point; return remind_cnt; } @@ -917,7 +915,6 @@ void rtl92ee_set_desc(struct ieee80211_hw *hw, u8 *pdesc, bool istx, struct rtl_priv *rtlpriv = rtl_priv(hw); u16 cur_tx_rp = 0; u16 cur_tx_wp = 0; - static u16 last_txw_point; static bool over_run; u32 tmp = 0; u8 q_idx = *val; @@ -951,9 +948,6 @@ void rtl92ee_set_desc(struct ieee80211_hw *hw, u8 *pdesc, bool istx, rtl_write_word(rtlpriv, get_desc_addr_fr_q_idx(q_idx), ring->cur_tx_wp); - - if (q_idx == 1) - last_txw_point = cur_tx_wp; } if (ring->avl_desc < (max_tx_desc - 15)) { -- 2.5.0
[PATCH] wireless: airo: remove unnecessary static in writerids()
Remove unnecessary static on local function pointer _writer_. Such pointer is initialized before being used, on every execution path throughout the function. The static has no benefit and, removing it reduces the object file size. This issue was detected using Coccinelle and the following semantic patch: @bad exists@ position p; identifier x; type T; @@ static T x@p; ... x = <+...x...+> @@ identifier x; expression e; type T; position p != bad.p; @@ -static T x@p; ... when != x when strict ?x = e; In the following log you can see a significant difference in the object file size. This log is the output of the size command, before and after the code change: before: textdata bss dec hex filename 113797 191521216 134165 20c15 drivers/net/wireless/cisco/airo.o after: textdata bss dec hex filename 113881 190961152 134129 20bf1 drivers/net/wireless/cisco/airo.o Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/cisco/airo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c index 84143a0..1066d84 100644 --- a/drivers/net/wireless/cisco/airo.c +++ b/drivers/net/wireless/cisco/airo.c @@ -7837,7 +7837,7 @@ static int writerids(struct net_device *dev, aironet_ioctl *comp) { struct airo_info *ai = dev->ml_priv; int ridcode; int enabled; - static int (* writer)(struct airo_info *, u16 rid, const void *, int, int); + int (*writer)(struct airo_info *, u16 rid, const void *, int, int); unsigned char *iobuf; /* Only super-user can write RIDs */ -- 2.5.0
Re: ti: wl18xx: add checks on wl18xx_top_reg_write() return value
Quoting Kalle Valo : "Gustavo A. R. Silva" wrote: Check return value from call to wl18xx_top_reg_write(), so in case of error jump to goto label out and return. Also, remove unnecessary value check before goto label out. Addresses-Coverity-ID: 1226938 Signed-off-by: Gustavo A. R. Silva The prefix should be "wl18xx:", I'll fix that. Thanks, Kalle. -- Gustavo A. R. Silva
[PATCH] ath9k: remove useless variable assignment in ath_mci_intr()
Value assigned to variable offset at line 551 is overwritten at line 562, before it can be used. This makes such variable assignment useless. Addresses-Coverity-ID: 1226941 Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ath/ath9k/mci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath9k/mci.c b/drivers/net/wireless/ath/ath9k/mci.c index 66596b9..cf23fd8 100644 --- a/drivers/net/wireless/ath/ath9k/mci.c +++ b/drivers/net/wireless/ath/ath9k/mci.c @@ -548,7 +548,7 @@ void ath_mci_intr(struct ath_softc *sc) if (mci_int_rxmsg & AR_MCI_INTERRUPT_RX_MSG_SCHD_INFO) { mci_int_rxmsg &= ~AR_MCI_INTERRUPT_RX_MSG_SCHD_INFO; - offset = ar9003_mci_state(ah, MCI_STATE_LAST_SCHD_MSG_OFFSET); + ar9003_mci_state(ah, MCI_STATE_LAST_SCHD_MSG_OFFSET); } if (mci_int_rxmsg & AR_MCI_INTERRUPT_RX_MSG_GPM) { -- 2.5.0
[PATCH] ti: wl18xx: add checks on wl18xx_top_reg_write() return value
Check return value from call to wl18xx_top_reg_write(), so in case of error jump to goto label out and return. Also, remove unnecessary value check before goto label out. Addresses-Coverity-ID: 1226938 Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/ti/wl18xx/main.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ti/wl18xx/main.c b/drivers/net/wireless/ti/wl18xx/main.c index d1aa3ee..0cf3b40 100644 --- a/drivers/net/wireless/ti/wl18xx/main.c +++ b/drivers/net/wireless/ti/wl18xx/main.c @@ -793,9 +793,13 @@ static int wl18xx_set_clk(struct wl1271 *wl) ret = wl18xx_top_reg_write(wl, PLLSH_WCS_PLL_P_FACTOR_CFG_2, (wl18xx_clk_table[clk_freq].p >> 16) & PLLSH_WCS_PLL_P_FACTOR_CFG_2_MASK); + if (ret < 0) + goto out; } else { ret = wl18xx_top_reg_write(wl, PLLSH_WCS_PLL_SWALLOW_EN, PLLSH_WCS_PLL_SWALLOW_EN_VAL2); + if (ret < 0) + goto out; } /* choose WCS PLL */ @@ -819,8 +823,6 @@ static int wl18xx_set_clk(struct wl1271 *wl) /* reset the swallowing logic */ ret = wl18xx_top_reg_write(wl, PLLSH_COEX_PLL_SWALLOW_EN, PLLSH_COEX_PLL_SWALLOW_EN_VAL2); - if (ret < 0) - goto out; out: return ret; -- 2.5.0
Re: [PATCH] nfc: nci: remove unnecessary null check
Hi Samuel, Quoting Samuel Ortiz : Hi Gustavo, On Tue, Jun 13, 2017 at 11:37:18AM -0500, Gustavo A. R. Silva wrote: Remove unnecessary NULL check for pointer conn_info. conn_info is set in list_for_each_entry() using container_of(), which is never NULL. Addresses-Coverity-ID: 1362349 Cc: Guenter Roeck Signed-off-by: Gustavo A. R. Silva --- net/nfc/nci/core.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) Applied to nfc-next, thanks. Absolutely, glad to help. Thanks -- Gustavo A. R. Silva
[PATCH] rtlwifi: rtl8821ae: remove unused variable
Remove unused variable rtlhal. Addresses-Coverity-ID: 1248810 Signed-off-by: Gustavo A. R. Silva --- drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c index 2bc6bac..d158e34 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c @@ -1360,7 +1360,6 @@ static bool _rtl8821ae_reset_pcie_interface_dma(struct ieee80211_hw *hw, static void _rtl8821ae_get_wakeup_reason(struct ieee80211_hw *hw) { struct rtl_priv *rtlpriv = rtl_priv(hw); - struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw)); struct rtl_ps_ctl *ppsc = rtl_psc(rtlpriv); u8 fw_reason = 0; struct timeval ts; @@ -1372,8 +1371,6 @@ static void _rtl8821ae_get_wakeup_reason(struct ieee80211_hw *hw) ppsc->wakeup_reason = 0; - rtlhal->last_suspend_sec = ts.tv_sec; - switch (fw_reason) { case FW_WOW_V2_PTK_UPDATE_EVENT: ppsc->wakeup_reason = WOL_REASON_PTK_UPDATE; -- 2.5.0
[PATCH] nfc: nci: remove unnecessary null check
Remove unnecessary NULL check for pointer conn_info. conn_info is set in list_for_each_entry() using container_of(), which is never NULL. Addresses-Coverity-ID: 1362349 Cc: Guenter Roeck Signed-off-by: Gustavo A. R. Silva --- net/nfc/nci/core.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c index 61fff42..c15cb88 100644 --- a/net/nfc/nci/core.c +++ b/net/nfc/nci/core.c @@ -73,11 +73,10 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, if (conn_info->dest_type == dest_type) { if (!params) return conn_info->conn_id; - if (conn_info) { - if (params->id == conn_info->dest_params->id && - params->protocol == conn_info->dest_params->protocol) - return conn_info->conn_id; - } + + if (params->id == conn_info->dest_params->id && + params->protocol == conn_info->dest_params->protocol) + return conn_info->conn_id; } } -- 2.5.0
[PATCH] nfc: nci: fix potential NULL pointer dereference
NULL check at line 76: if (conn_info) {, implies that pointer conn_info might be NULL, but this pointer is being previously dereferenced, which might cause a NULL pointer dereference. Add NULL check before dereferencing pointer conn_info in order to avoid a potential NULL pointer dereference. Addresses-Coverity-ID: 1362349 Signed-off-by: Gustavo A. R. Silva --- net/nfc/nci/core.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c index 61fff42..d2198ce 100644 --- a/net/nfc/nci/core.c +++ b/net/nfc/nci/core.c @@ -70,14 +70,13 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, struct nci_conn_info *conn_info; list_for_each_entry(conn_info, &ndev->conn_info_list, list) { - if (conn_info->dest_type == dest_type) { + if (conn_info && conn_info->dest_type == dest_type) { if (!params) return conn_info->conn_id; - if (conn_info) { - if (params->id == conn_info->dest_params->id && - params->protocol == conn_info->dest_params->protocol) - return conn_info->conn_id; - } + + if (params->id == conn_info->dest_params->id && + params->protocol == conn_info->dest_params->protocol) + return conn_info->conn_id; } } -- 2.5.0