linux-kernel - Re: [PATCH] ath9k: unify error handling code in ath9k_hif_usb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87v8cqs165.fsf@toke.dk>
Date:   Mon, 04 Sep 2023 12:06:10 +0200
From:   Toke Høiland-Jørgensen <toke@...e.dk>
To:     Dongliang Mu <dzm91@...t.edu.cn>, Kalle Valo <kvalo@...nel.org>
Cc:     hust-os-kernel-patches@...glegroups.com,
        Dongliang Mu <dzm91@...t.edu.cn>,
        linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ath9k: unify error handling code in ath9k_hif_usb_resume

Dongliang Mu <dzm91@...t.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@...t.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