[PATCH] ath9k: unify error handling code in ath9k_hif_usb_resume

[PATCH] ath9k: unify error handling code in ath9k_hif_usb_resume

[Dongliang Mu]

In ath9k_hif_usb_resume, the error handling code calls
ath9k_hif_usb_dealloc_urbs twice in different paths.

To unify the error handling code, we replace one error handling path
with a goto statement.

Note that this patch does not incur any functionability change.

Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index e5414435b141..dcc01274b008 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -1502,8 +1502,8 @@ static int ath9k_hif_usb_resume(struct usb_interface *interface)
 		if (ret)
 			goto fail_resume;
 	} else {
-		ath9k_hif_usb_dealloc_urbs(hif_dev);
-		return -EIO;
+		ret = -EIO;
+		goto fail_resume;
 	}
 
 	mdelay(100);
-- 
2.25.1

Re: [PATCH] ath9k: unify error handling code in ath9k_hif_usb_resume

[Toke Høiland-Jørgensen]

Dongliang Mu <dzm91@hust.edu.cn> writes:

> In ath9k_hif_usb_resume, the error handling code calls
> ath9k_hif_usb_dealloc_urbs twice in different paths.
>
> To unify the error handling code, we replace one error handling path
> with a goto statement.
>
> Note that this patch does not incur any functionability change.
>
> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>

Hmm, if you're cleaning up that function, how about changing that else
to an early error return? I.e. change the if at the top to:

	if (!(hif_dev->flags & HIF_USB_READY)) {
		ret = -EIO;
		goto fail_resume;
	}

and drop one level of indentation from what is currently in the top
branch of the if statement.

Also, while you're at it, please reorder the variable declarations at
the top of the function to be reverse x-mas tree order (moving the 'int
ret' declaration to the bottom).

-Toke

Re: [PATCH] ath9k: unify error handling code in ath9k_hif_usb_resume

[Dongliang Mu]

On 9/4/23 18:06, Toke Høiland-Jørgensen wrote:
> Dongliang Mu <dzm91@hust.edu.cn> writes:
>
>> In ath9k_hif_usb_resume, the error handling code calls
>> ath9k_hif_usb_dealloc_urbs twice in different paths.
>>
>> To unify the error handling code, we replace one error handling path
>> with a goto statement.
>>
>> Note that this patch does not incur any functionability change.
>>
>> Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
> Hmm, if you're cleaning up that function, how about changing that else
> to an early error return? I.e. change the if at the top to:
>
> 	if (!(hif_dev->flags & HIF_USB_READY)) {
> 		ret = -EIO;
> 		goto fail_resume;
> 	}
>
> and drop one level of indentation from what is currently in the top
> branch of the if statement.

Yeah, this is more elegant. I've sent a patch v2.

Dongliang Mu

>
> Also, while you're at it, please reorder the variable declarations at
> the top of the function to be reverse x-mas tree order (moving the 'int
> ret' declaration to the bottom).
>
> -Toke