| Message ID | 7d5cc7b4-f0ce-192a-3e5e-c2944c1e3e3d@users.sourceforge.net (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Kalle Valo |
| Headers | show |
On 3/10/2018 10:33 PM, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Sat, 10 Mar 2018 22:18:45 +0100 > > Add a jump target so that a bit of exception handling can be better reused > at the end of this function. > > This issue was detected by using the Coccinelle software. You call this an issue? > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/net/wireless/ti/wlcore/acx.c | 24 +++++++++++------------- > 1 file changed, 11 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c > index 1cc5bba670e1..7d37a417c756 100644 > --- a/drivers/net/wireless/ti/wlcore/acx.c > +++ b/drivers/net/wireless/ti/wlcore/acx.c [...] > ret = wl1271_cmd_configure(wl, ACX_RATE_POLICY, acx, sizeof(*acx)); > - if (ret < 0) { > - wl1271_warning("Setting of rate policies failed: %d", ret); > - goto out; > - } > + if (ret < 0) > + goto report_failure; > > -out: > +free_acx: > kfree(acx); > return ret; > + > +report_failure: > + wl1271_warning("Setting of rate policies failed: %d", ret); > + goto free_acx; In my opinion you are introducing a new issue. I don't call this "common" in any way. It is leaning more towards "spaghetti code" [1]. Jumping over a label to return to it with another jump. They are not long jumps, but it sure does not make thing more readable. Always aim for simple top-to-bottom. Regards, Arend [1] https://en.wikipedia.org/wiki/Spaghetti_code > } > > int wl1271_acx_ap_rate_policy(struct wl1271 *wl, struct conf_tx_rate_class *c, >
diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c index 1cc5bba670e1..7d37a417c756 100644 --- a/drivers/net/wireless/ti/wlcore/acx.c +++ b/drivers/net/wireless/ti/wlcore/acx.c @@ -757,10 +757,8 @@ int wl1271_acx_sta_rate_policies(struct wl1271 *wl, struct wl12xx_vif *wlvif) acx->rate_policy.aflags = c->aflags; ret = wl1271_cmd_configure(wl, ACX_RATE_POLICY, acx, sizeof(*acx)); - if (ret < 0) { - wl1271_warning("Setting of rate policies failed: %d", ret); - goto out; - } + if (ret < 0) + goto report_failure; /* configure one AP supported rate class */ acx->rate_policy_idx = cpu_to_le32(wlvif->sta.ap_rate_idx); @@ -773,10 +771,8 @@ int wl1271_acx_sta_rate_policies(struct wl1271 *wl, struct wl12xx_vif *wlvif) acx->rate_policy.aflags = c->aflags; ret = wl1271_cmd_configure(wl, ACX_RATE_POLICY, acx, sizeof(*acx)); - if (ret < 0) { - wl1271_warning("Setting of rate policies failed: %d", ret); - goto out; - } + if (ret < 0) + goto report_failure; /* * configure one rate class for basic p2p operations. @@ -791,14 +787,16 @@ int wl1271_acx_sta_rate_policies(struct wl1271 *wl, struct wl12xx_vif *wlvif) acx->rate_policy.aflags = c->aflags; ret = wl1271_cmd_configure(wl, ACX_RATE_POLICY, acx, sizeof(*acx)); - if (ret < 0) { - wl1271_warning("Setting of rate policies failed: %d", ret); - goto out; - } + if (ret < 0) + goto report_failure; -out: +free_acx: kfree(acx); return ret; + +report_failure: + wl1271_warning("Setting of rate policies failed: %d", ret); + goto free_acx; } int wl1271_acx_ap_rate_policy(struct wl1271 *wl, struct conf_tx_rate_class *c,