Skip to contentSkip to conversation

wireless: mark expected switch fall-throughs

PATCH In review Gustavo A. R. Silva 2 participants 3 messages Jul 4, 2018 – Oct 23, 2018

Subsystems (3)
Networking Network drivers 802.11 (including cfg80211/NL80211)
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 wireless: mark expected switch fall-throughs

In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.

Warning level 2 was used: -Wimplicit-fallthrough=2

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/wireless/chan.c        | 2 ++
 net/wireless/nl80211.c     | 9 +++++++++
 net/wireless/wext-compat.c | 2 ++
 3 files changed, 13 insertions(+)
net/wireless/chan.c +2 -0
index 2db713d..145cb51 100644
@@ -747,6 +747,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
 	case NL80211_CHAN_WIDTH_20:
 		if (!ht_cap->ht_supported)
 			return false;
+		/* else: 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;
+		/* else: fall through */
 	case NL80211_CHAN_WIDTH_80:
 		if (!vht_cap->vht_supported)
 			return false;
net/wireless/nl80211.c +9 -0
index e4e5f02..59b0ac5 100644
@@ -1676,6 +1676,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* else: fall through */
 	case 1:
 		if (nla_put(msg, NL80211_ATTR_CIPHER_SUITES,
 			    sizeof(u32) * rdev->wiphy.n_cipher_suites,

@@ -1722,6 +1723,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* else: fall through */
 	case 2:
 		if (nl80211_put_iftypes(msg, NL80211_ATTR_SUPPORTED_IFTYPES,
 					rdev->wiphy.interface_modes))

@@ -1729,6 +1731,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* else: fall through */
 	case 3:
 		nl_bands = nla_nest_start(msg, NL80211_ATTR_WIPHY_BANDS);
 		if (!nl_bands)

@@ -1754,6 +1757,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 				state->chan_start++;
 				if (state->split)
 					break;
+				/* else: fall through */
 			default:
 				/* add frequencies */
 				nl_freqs = nla_nest_start(

@@ -1807,6 +1811,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 			state->split_start++;
 		if (state->split)
 			break;
+		/* else: fall through */
 	case 4:
 		nl_cmds = nla_nest_start(msg, NL80211_ATTR_SUPPORTED_COMMANDS);
 		if (!nl_cmds)

@@ -1833,6 +1838,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* else: fall through */
 	case 5:
 		if (rdev->ops->remain_on_channel &&
 		    (rdev->wiphy.flags & WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL) &&

@@ -1850,6 +1856,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* else: fall through */
 	case 6:
 #ifdef CONFIG_PM
 		if (nl80211_send_wowlan(msg, rdev, state->split))

@@ -1872,6 +1879,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* else: fall through */
 	case 8:
 		if ((rdev->wiphy.flags & WIPHY_FLAG_HAVE_AP_SME) &&
 		    nla_put_u32(msg, NL80211_ATTR_DEVICE_AP_SME,

@@ -4466,6 +4474,7 @@ static int parse_station_flags(struct genl_info *info,
 		params->sta_flags_mask = BIT(NL80211_STA_FLAG_AUTHENTICATED) |
 					 BIT(NL80211_STA_FLAG_MFP) |
 					 BIT(NL80211_STA_FLAG_AUTHORIZED);
+		/* fall through */
 	default:
 		return -EINVAL;
 	}
net/wireless/wext-compat.c +2 -0
index 167f702..9db3920 100644
@@ -1333,6 +1333,7 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev)
 			wstats.qual.qual = sig + 110;
 			break;
 		}
+		/* else: fall through */
 	case CFG80211_SIGNAL_TYPE_UNSPEC:
 		if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_SIGNAL)) {
 			wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED;

@@ -1341,6 +1342,7 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev)
 			wstats.qual.qual = sinfo.signal;
 			break;
 		}
+		/* else: fall through */
 	default:
 		wstats.qual.updated |= IW_QUAL_LEVEL_INVALID;
 		wstats.qual.updated |= IW_QUAL_QUAL_INVALID;
Signature · 1 line
2.7.4
maintainer
#1 lore raw

Hi Gustavo,

Quoted 2 lines

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?

net/wireless/chan.c bool cfg80211_chandef_usable(struct wiphy *wiphy, · line 750

Quoted 4 lines
net/wireless/chan.c +1 -0
 	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:"

johannes

author
#2 lore raw
Quoted 7 lines · Johannes Berg

Hi Gustavo,

Quoted 2 lines

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.

Quoted 14 lines

net/wireless/chan.c bool cfg80211_chandef_usable(struct wiphy *wiphy, · line 750

Quoted 4 lines
net/wireless/chan.c +1 -0
 	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();

Quoted 8 lines

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