Search Linux Wireless

Re: [PATCH 3/3] wlcore: Use common error handling code in wl1271_acx_sta_rate_policies()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/10/2018 10:33 PM, SF Markus Elfring wrote:
From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx>
---
  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,





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux