| Message ID | 20240619040959.1457547-7-quic_adisi@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: cfg80211/mac80211: add DFS support for MLO | expand |
On Wed, 2024-06-19 at 09:39 +0530, Aditya Kumar Singh wrote: > After locks rework [1], ieee80211_start_radar_detection() function is no > longer acquiring any lock as such explicitly. Hence, it is not unlocking > anything as well. However, label "out_unlock" is still used which creates > confusion. > > Rename the label to "return_err". Probably better to get rid of it entirely? > > [1]: https://lore.kernel.org/all/20230828135928.b1c6efffe9ad.I4aec875e25abc9ef0b5ad1e70b5747fd483fbd3c@changeid/ I _think_ people were suggesting to drop the ":" from that? > @@ -3477,7 +3477,7 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy, > > if (!list_empty(&local->roc_list) || local->scanning) { > err = -EBUSY; > - goto out_unlock; > + goto return_err; can drop braces, "return -EBUSY;" > @@ -3487,12 +3487,12 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy, > err = ieee80211_link_use_channel(&sdata->deflink, &chanreq, > IEEE80211_CHANCTX_SHARED); > if (err) > - goto out_unlock; > + goto return_err; return err; > wiphy_delayed_work_queue(wiphy, &sdata->deflink.dfs_cac_timer_work, > msecs_to_jiffies(cac_time_ms)); > > - out_unlock: > + return_err: > return err; > and that can then become "return 0" which is much nicer anyway Cf. also https://staticthinking.wordpress.com/2024/02/28/return-0-is-better-than-return-ret/ johannes
On 6/25/24 17:30, Johannes Berg wrote: > On Wed, 2024-06-19 at 09:39 +0530, Aditya Kumar Singh wrote: >> After locks rework [1], ieee80211_start_radar_detection() function is no >> longer acquiring any lock as such explicitly. Hence, it is not unlocking >> anything as well. However, label "out_unlock" is still used which creates >> confusion. >> >> Rename the label to "return_err". > > Probably better to get rid of it entirely? Yup, good point. Will do in next version. Thanks for the input. >> >> [1]: https://lore.kernel.org/all/20230828135928.b1c6efffe9ad.I4aec875e25abc9ef0b5ad1e70b5747fd483fbd3c@changeid/ > > I _think_ people were suggesting to drop the ":" from that? > >> @@ -3477,7 +3477,7 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy, >> >> if (!list_empty(&local->roc_list) || local->scanning) { >> err = -EBUSY; >> - goto out_unlock; >> + goto return_err; > > can drop braces, "return -EBUSY;" That's correct. > >> @@ -3487,12 +3487,12 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy, >> err = ieee80211_link_use_channel(&sdata->deflink, &chanreq, >> IEEE80211_CHANCTX_SHARED); >> if (err) >> - goto out_unlock; >> + goto return_err; > > return err; Yep. > >> wiphy_delayed_work_queue(wiphy, &sdata->deflink.dfs_cac_timer_work, >> msecs_to_jiffies(cac_time_ms)); >> >> - out_unlock: >> + return_err: >> return err; >> > > and that can then become "return 0" which is much nicer anyway > > Cf. also > https://staticthinking.wordpress.com/2024/02/28/return-0-is-better-than-return-ret/ > Indeed! Will do. Thanks.
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index c2af768add96..b2cd43d807ed 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -3477,7 +3477,7 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy, if (!list_empty(&local->roc_list) || local->scanning) { err = -EBUSY; - goto out_unlock; + goto return_err; } /* whatever, but channel contexts should not complain about that one */ @@ -3487,12 +3487,12 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy, err = ieee80211_link_use_channel(&sdata->deflink, &chanreq, IEEE80211_CHANCTX_SHARED); if (err) - goto out_unlock; + goto return_err; wiphy_delayed_work_queue(wiphy, &sdata->deflink.dfs_cac_timer_work, msecs_to_jiffies(cac_time_ms)); - out_unlock: + return_err: return err; }
After locks rework [1], ieee80211_start_radar_detection() function is no longer acquiring any lock as such explicitly. Hence, it is not unlocking anything as well. However, label "out_unlock" is still used which creates confusion. Rename the label to "return_err". [1]: https://lore.kernel.org/all/20230828135928.b1c6efffe9ad.I4aec875e25abc9ef0b5ad1e70b5747fd483fbd3c@changeid/ Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com> --- net/mac80211/cfg.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)