| Message ID | 20201019150507.20574-1-trix@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | wireless: remove unneeded break | expand |
On 19/10/2020 17:05, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > A break is not needed if it is preceded by a return or goto > > Signed-off-by: Tom Rix <trix@redhat.com> > diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c > index 5bd35c147e19..3ca9d26df174 100644 > --- a/drivers/net/wireless/intersil/p54/eeprom.c > +++ b/drivers/net/wireless/intersil/p54/eeprom.c > @@ -870,7 +870,6 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) > } else { > goto good_eeprom; > } > - break; Won't the compiler (gcc) now complain about a missing fallthrough annotation? > default: > break; > } Cheers, Christian
On Mon, 2020-10-19 at 17:14 +0200, Christian Lamparter wrote: > On 19/10/2020 17:05, trix@redhat.com wrote: > > From: Tom Rix <trix@redhat.com> > > > > A break is not needed if it is preceded by a return or goto > > > > Signed-off-by: Tom Rix <trix@redhat.com> > > diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c > > index 5bd35c147e19..3ca9d26df174 100644 > > --- a/drivers/net/wireless/intersil/p54/eeprom.c > > +++ b/drivers/net/wireless/intersil/p54/eeprom.c > > @@ -870,7 +870,6 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) > > } else { > > goto good_eeprom; > > } > > - break; > Won't the compiler (gcc) now complain about a missing fallthrough annotation? > > default: > > break; > > } No, though the code would be clearer like: --- diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c index 5bd35c147e19..233fa072d96d 100644 --- a/drivers/net/wireless/intersil/p54/eeprom.c +++ b/drivers/net/wireless/intersil/p54/eeprom.c @@ -867,10 +867,8 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) "test!\n"); err = -ENOMSG; goto err; - } else { - goto good_eeprom; } - break; + goto good_eeprom; default: break; }
On 10/19/20 10:21, Joe Perches wrote: > On Mon, 2020-10-19 at 17:14 +0200, Christian Lamparter wrote: >> On 19/10/2020 17:05, trix@redhat.com wrote: >>> From: Tom Rix <trix@redhat.com> >>> >>> A break is not needed if it is preceded by a return or goto >>> >>> Signed-off-by: Tom Rix <trix@redhat.com> >>> diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c >>> index 5bd35c147e19..3ca9d26df174 100644 >>> --- a/drivers/net/wireless/intersil/p54/eeprom.c >>> +++ b/drivers/net/wireless/intersil/p54/eeprom.c >>> @@ -870,7 +870,6 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) >>> } else { >>> goto good_eeprom; >>> } >>> - break; >> Won't the compiler (gcc) now complain about a missing fallthrough annotation? Clang would definitely complain about this. >>> default: >>> break; >>> } > > No, though the code would be clearer like: > --- > diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c > index 5bd35c147e19..233fa072d96d 100644 > --- a/drivers/net/wireless/intersil/p54/eeprom.c > +++ b/drivers/net/wireless/intersil/p54/eeprom.c > @@ -867,10 +867,8 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) > "test!\n"); > err = -ENOMSG; > goto err; > - } else { > - goto good_eeprom; > } > - break; > + goto good_eeprom; > default: > break; > } > > This is much better because it'd avoid any complain from Clang. -- Gustavo
On Mon, 2020-10-19 at 10:54 -0500, Gustavo A. R. Silva wrote: > On 10/19/20 10:21, Joe Perches wrote: > > On Mon, 2020-10-19 at 17:14 +0200, Christian Lamparter wrote: > > > On 19/10/2020 17:05, trix@redhat.com wrote: > > > > From: Tom Rix <trix@redhat.com> > > > > > > > > A break is not needed if it is preceded by a return or goto > > > > > > > > Signed-off-by: Tom Rix <trix@redhat.com> > > > > diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c [] > > > > @@ -870,7 +870,6 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) > > > > } else { > > > > goto good_eeprom; > > > > } > > > > - break; > > > Won't the compiler (gcc) now complain about a missing fallthrough annotation? > > Clang would definitely complain about this. As far as I can tell, clang 10.0.0 doesn't complain. This compiles without fallthrough complaint from make V=1 W=123 CC=clang drivers/net/wireless/intersil/p54/eeprom.o with -Wimplicit-fallthrough added $ clang -Wp,-MMD,drivers/net/wireless/intersil/p54/.eeprom.o.d -nostdinc -isystem /usr/local/lib/clang/10.0.0/include -I./arch/x86/include -I./arch/x86/include/generated -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -Qunused-arguments -DKBUILD_EXTRA_WARN1 -DKBUILD_EXTRA_WARN2 -DKBUILD_EXTRA_WARN3 -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 -no-integrated-as -Werror=unknown-warning-option -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 -mno-80387 -mstack-alignment=8 -mtune=generic -mno-red-zone -mcmodel=kernel -DCONFIG_X86_X32_ABI -Wno-sign-compare -fno-asynchronous-unwind-tables -mretpoline-external-thunk -fno-delete-null-pointer-checks -Wno-address-of-packed-mem ber -O2 -Wframe-larger-than=2048 -fstack-protector-strong -Wno-format-invalid-specifier -Wno-gnu -mno-global-merge -Wno-unused-const-variable -ftrivial-auto-var-init=pattern -pg -mfentry -DCC_USING_FENTRY -falign-functions=32 -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-array-bounds -fno-strict-overflow -fno-stack-check -Werror=date-time -Werror=incompatible-pointer-types -fcf-protection=none -Wextra -Wunused -Wno-unused-parameter -Wmissing-declarations -Wmissing-format-attribute -Wmissing-prototypes -Wold-style-definition -Wmissing-include-dirs -Wunused-const-variable -Wno-missing-field-initializers -Wno-sign-compare -Wno-type-limits -Wcast-align -Wdisabled-optimization -Wnested-externs -Wshadow -Wmissing-field-initializers -Wtype-limits -Wunused-macros -Wbad-function-cast -Wcast-qual -Wconversion -Wpacked -Wpadded -Wpointer-arith -Wredundant-decls -Wsign-compare -Wswitch-default -fsanitize=kernel-address -mllvm -asan-mapping-offset=0xdffffc0000000000 -mllvm -asa n-globals=1 -mllvm -asan-instrumentation-with-call-threshold=0 -mllvm -asan-stack=0 --param asan-instrument-allocas=1 -fsanitize-coverage=trace-pc -fsanitize-coverage=trace-cmp -Wimplicit-fallthrough -DKBUILD_MODFILE='"drivers/net/wireless/intersil/p54/p54common"' -DKBUILD_BASENAME='"eeprom"' -DKBUILD_MODNAME='"p54common"' -c -o drivers/net/wireless/intersil/p54/eeprom.o drivers/net/wireless/intersil/p54/eeprom.c
On 10/19/20 9:20 AM, Joe Perches wrote: > On Mon, 2020-10-19 at 10:54 -0500, Gustavo A. R. Silva wrote: >> On 10/19/20 10:21, Joe Perches wrote: >>> On Mon, 2020-10-19 at 17:14 +0200, Christian Lamparter wrote: >>>> On 19/10/2020 17:05, trix@redhat.com wrote: >>>>> From: Tom Rix <trix@redhat.com> >>>>> >>>>> A break is not needed if it is preceded by a return or goto >>>>> >>>>> Signed-off-by: Tom Rix <trix@redhat.com> >>>>> diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c > [] >>>>> @@ -870,7 +870,6 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) >>>>> } else { >>>>> goto good_eeprom; >>>>> } >>>>> - break; >>>> Won't the compiler (gcc) now complain about a missing fallthrough annotation? >> Clang would definitely complain about this. > As far as I can tell, clang 10.0.0 doesn't complain. > > This compiles without fallthrough complaint > > from make V=1 W=123 CC=clang drivers/net/wireless/intersil/p54/eeprom.o > with -Wimplicit-fallthrough added > > $ clang -Wp,-MMD,drivers/net/wireless/intersil/p54/.eeprom.o.d -nostdinc -isystem /usr/local/lib/clang/10.0.0/include -I./arch/x86/include -I./arch/x86/include/generated -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -Qunused-arguments -DKBUILD_EXTRA_WARN1 -DKBUILD_EXTRA_WARN2 -DKBUILD_EXTRA_WARN3 -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 -no-integrated-as -Werror=unknown-warning-option -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -m64 -mno-80387 -mstack-alignment=8 -mtune=generic -mno-red-zone -mcmodel=kernel -DCONFIG_X86_X32_ABI -Wno-sign-compare -fno-asynchronous-unwind-tables -mretpoline-external-thunk -fno-delete-null-pointer-checks -Wno-address-of-packed-m ember -O2 -Wframe-larger-than=2048 -fstack-protector-strong -Wno-format-invalid-specifier -Wno-gnu -mno-global-merge -Wno-unused-const-variable -ftrivial-auto-var-init=pattern -pg -mfentry -DCC_USING_FENTRY -falign-functions=32 -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-array-bounds -fno-strict-overflow -fno-stack-check -Werror=date-time -Werror=incompatible-pointer-types -fcf-protection=none -Wextra -Wunused -Wno-unused-parameter -Wmissing-declarations -Wmissing-format-attribute -Wmissing-prototypes -Wold-style-definition -Wmissing-include-dirs -Wunused-const-variable -Wno-missing-field-initializers -Wno-sign-compare -Wno-type-limits -Wcast-align -Wdisabled-optimization -Wnested-externs -Wshadow -Wmissing-field-initializers -Wtype-limits -Wunused-macros -Wbad-function-cast -Wcast-qual -Wconversion -Wpacked -Wpadded -Wpointer-arith -Wredundant-decls -Wsign-compare -Wswitch-default -fsanitize=kernel-address -mllvm -asan-mapping-offset=0xdffffc0000000000 -mllvm -a san-globals=1 -mllvm -asan-instrumentation-with-call-threshold=0 -mllvm -asan-stack=0 --param asan-instrument-allocas=1 -fsanitize-coverage=trace-pc -fsanitize-coverage=trace-cmp -Wimplicit-fallthrough -DKBUILD_MODFILE='"drivers/net/wireless/intersil/p54/p54common"' -DKBUILD_BASENAME='"eeprom"' -DKBUILD_MODNAME='"p54common"' -c -o drivers/net/wireless/intersil/p54/eeprom.o drivers/net/wireless/intersil/p54/eeprom.c > I did not intend for if-else; break; changes to be in the patchset. I will kick this out and respin the patch after i get the first pass of the other changes out. Sorry, Tom
On 10/19/20 11:20, Joe Perches wrote: > On Mon, 2020-10-19 at 10:54 -0500, Gustavo A. R. Silva wrote: >> On 10/19/20 10:21, Joe Perches wrote: >>> On Mon, 2020-10-19 at 17:14 +0200, Christian Lamparter wrote: >>>> On 19/10/2020 17:05, trix@redhat.com wrote: >>>>> From: Tom Rix <trix@redhat.com> >>>>> >>>>> A break is not needed if it is preceded by a return or goto >>>>> >>>>> Signed-off-by: Tom Rix <trix@redhat.com> >>>>> diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c > [] >>>>> @@ -870,7 +870,6 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) >>>>> } else { >>>>> goto good_eeprom; >>>>> } >>>>> - break; >>>> Won't the compiler (gcc) now complain about a missing fallthrough annotation? >> >> Clang would definitely complain about this. > > As far as I can tell, clang 10.0.0 doesn't complain. Oh, yeah. I didn't see the other "goto err;" in the if clause above. Clang doesn't complain because there is actually no chance of any implicit fall-through. -- Gustavo
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c index 5c1af2021883..9c4e6cf2137a 100644 --- a/drivers/net/wireless/ath/ath10k/htt_rx.c +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c @@ -3878,7 +3878,6 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb) return ath10k_htt_rx_proc_rx_frag_ind(htt, &resp->rx_frag_ind, skb); - break; } case HTT_T2H_MSG_TYPE_TEST: break; diff --git a/drivers/net/wireless/ath/ath6kl/testmode.c b/drivers/net/wireless/ath/ath6kl/testmode.c index f3906dbe5495..89c7c4e25169 100644 --- a/drivers/net/wireless/ath/ath6kl/testmode.c +++ b/drivers/net/wireless/ath/ath6kl/testmode.c @@ -94,7 +94,6 @@ int ath6kl_tm_cmd(struct wiphy *wiphy, struct wireless_dev *wdev, return 0; - break; case ATH6KL_TM_CMD_RX_REPORT: default: return -EOPNOTSUPP; diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c index 6609ce122e6e..b66eeb577272 100644 --- a/drivers/net/wireless/ath/ath9k/hw.c +++ b/drivers/net/wireless/ath/ath9k/hw.c @@ -2308,7 +2308,6 @@ void ath9k_hw_beaconinit(struct ath_hw *ah, u32 next_beacon, u32 beacon_period) ath_dbg(ath9k_hw_common(ah), BEACON, "%s: unsupported opmode: %d\n", __func__, ah->opmode); return; - break; } REG_WRITE(ah, AR_BEACON_PERIOD, beacon_period); diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c index cbdebefb854a..8698ca4d30de 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c @@ -1202,13 +1202,11 @@ static int iwl_mvm_mac_ctx_send(struct iwl_mvm *mvm, struct ieee80211_vif *vif, return iwl_mvm_mac_ctxt_cmd_sta(mvm, vif, action, force_assoc_off, bssid_override); - break; case NL80211_IFTYPE_AP: if (!vif->p2p) return iwl_mvm_mac_ctxt_cmd_ap(mvm, vif, action); else return iwl_mvm_mac_ctxt_cmd_go(mvm, vif, action); - break; case NL80211_IFTYPE_MONITOR: return iwl_mvm_mac_ctxt_cmd_listener(mvm, vif, action); case NL80211_IFTYPE_P2P_DEVICE: diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c index 5bd35c147e19..3ca9d26df174 100644 --- a/drivers/net/wireless/intersil/p54/eeprom.c +++ b/drivers/net/wireless/intersil/p54/eeprom.c @@ -870,7 +870,6 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len) } else { goto good_eeprom; } - break; default: break; } diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c index 63f9ea21962f..bd9160b166c5 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c @@ -1226,7 +1226,6 @@ static int _rtl88ee_set_media_status(struct ieee80211_hw *hw, default: pr_err("Network type %d not support!\n", type); return 1; - break; } /* MSR_INFRA == Link in infrastructure network; diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c index a36dc6e726d2..f8a1de6e9849 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c @@ -1132,7 +1132,6 @@ static int _rtl8723e_set_media_status(struct ieee80211_hw *hw, default: pr_err("Network type %d not support!\n", type); return 1; - break; } /* MSR_INFRA == Link in infrastructure network; diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c index f41a7643b9c4..225b8cd44f23 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c @@ -2085,12 +2085,10 @@ bool rtl8812ae_phy_config_rf_with_headerfile(struct ieee80211_hw *hw, return __rtl8821ae_phy_config_with_headerfile(hw, radioa_array_table_a, radioa_arraylen_a, _rtl8821ae_config_rf_radio_a); - break; case RF90_PATH_B: return __rtl8821ae_phy_config_with_headerfile(hw, radioa_array_table_b, radioa_arraylen_b, _rtl8821ae_config_rf_radio_b); - break; case RF90_PATH_C: case RF90_PATH_D: pr_err("switch case %#x not processed\n", rfpath); @@ -2116,7 +2114,6 @@ bool rtl8821ae_phy_config_rf_with_headerfile(struct ieee80211_hw *hw, return __rtl8821ae_phy_config_with_headerfile(hw, radioa_array_table, radioa_arraylen, _rtl8821ae_config_rf_radio_a); - break; case RF90_PATH_B: case RF90_PATH_C: