| Message ID | 20230903123230.2116129-1-dzm91@hust.edu.cn |
|---|---|
| State | New |
| Headers | show |
| Series | ath9k: unify error handling code in ath9k_hif_usb_resume | expand |
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
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
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);
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(-)