diff mbox

[1/5] wlcore: Use common error handling code in wlcore_nvs_cb()

Message ID 7b40d50f-cdac-bd47-5070-894140f7ceb3@users.sourceforge.net (mailing list archive)
State Rejected
Delegated to: Kalle Valo
Headers show

Commit Message

SF Markus Elfring Oct. 29, 2017, 8:11 p.m. UTC
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 29 Oct 2017 18:38:04 +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.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ti/wlcore/main.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Julian Calaby Oct. 30, 2017, 1:28 p.m. UTC | #1
Hi Markus,

On Mon, Oct 30, 2017 at 7:11 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 29 Oct 2017 18:38:04 +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.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/net/wireless/ti/wlcore/main.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index c346c021b999..48380d48ef09 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -6551,6 +6549,11 @@ static void wlcore_nvs_cb(const struct firmware *fw, void *context)
>  out:
>         release_firmware(fw);
>         complete_all(&wl->nvs_loading_complete);
> +       return;
> +
> +power_off:

Name this "out_power_off" to match the other labels.

> +       wl1271_power_off(wl);
> +       goto out_free_nvs;

Why not put this in front of the out_free_nvs label? It looks weird here.

Thanks,
SF Markus Elfring Oct. 30, 2017, 1:40 p.m. UTC | #2
>> @@ -6551,6 +6549,11 @@ static void wlcore_nvs_cb(const struct firmware *fw, void *context)
>>  out:
>>         release_firmware(fw);
>>         complete_all(&wl->nvs_loading_complete);
>> +       return;
>> +
>> +power_off:
> 
> Name this "out_power_off" to match the other labels.

Do you expect a second approach for this patch series then?


>> +       wl1271_power_off(wl);
>> +       goto out_free_nvs;
> 
> Why not put this in front of the out_free_nvs label?

It seems that I can not really follow this suggestion at the moment.


> It looks weird here.

Which detail do you not like?

Regards,
Markus
diff mbox

Patch

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index c346c021b999..48380d48ef09 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -6496,16 +6496,14 @@  static void wlcore_nvs_cb(const struct firmware *fw, void *context)
 	ret = wl12xx_get_hw_info(wl);
 	if (ret < 0) {
 		wl1271_error("couldn't get hw info");
-		wl1271_power_off(wl);
-		goto out_free_nvs;
+		goto power_off;
 	}
 
 	ret = request_threaded_irq(wl->irq, hardirq_fn, wlcore_irq,
 				   wl->irq_flags, pdev->name, wl);
 	if (ret < 0) {
 		wl1271_error("interrupt configuration failed");
-		wl1271_power_off(wl);
-		goto out_free_nvs;
+		goto power_off;
 	}
 
 #ifdef CONFIG_PM
@@ -6551,6 +6549,11 @@  static void wlcore_nvs_cb(const struct firmware *fw, void *context)
 out:
 	release_firmware(fw);
 	complete_all(&wl->nvs_loading_complete);
+	return;
+
+power_off:
+	wl1271_power_off(wl);
+	goto out_free_nvs;
 }
 
 int wlcore_probe(struct wl1271 *wl, struct platform_device *pdev)