Messages in this thread | | | Subject | Re: [PATCH] carl9170: Fix error return -EAGAIN if not started | From | Christian Lamparter <> | Date | Fri, 8 Oct 2021 17:15:35 +0200 |
| |
Hello,
On 08/10/2021 09:31, Colin Ian King wrote: > On 08/10/2021 06:58, Dan Carpenter wrote: >> On Fri, Oct 08, 2021 at 01:15:58AM +0100, Colin King wrote: >>> From: Colin Ian King <colin.king@canonical.com> >>> >>> There is an error return path where the error return is being >>> assigned to err rather than count and the error exit path does >>> not return -EAGAIN as expected. Fix this by setting the error >>> return to variable count as this is the value that is returned >>> at the end of the function. >>> >>> Addresses-Coverity: ("Unused value") >>> Fixes: 00c4da27a421 ("carl9170: firmware parser and debugfs code") >>> Signed-off-by: Colin Ian King <colin.king@canonical.com> >>> --- >>> drivers/net/wireless/ath/carl9170/debug.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c >>> index bb40889d7c72..f163c6bdac8f 100644 >>> --- a/drivers/net/wireless/ath/carl9170/debug.c >>> +++ b/drivers/net/wireless/ath/carl9170/debug.c >>> @@ -628,7 +628,7 @@ static ssize_t carl9170_debugfs_bug_write(struct ar9170 *ar, const char *buf, >>> case 'R': >>> if (!IS_STARTED(ar)) { >>> - err = -EAGAIN; >>> + count = -EAGAIN; >>> goto out; >> >> This is ugly. The bug wouldn't have happened with a direct return, it's >> only the goto out which causes it. Better to replace all the error >> paths with direct returns. There are two other direct returns so it's >> not like a new thing... > > Yep, I agree it was ugly, I was trying to keep to the coding style and reduce the patch delta size. I can do a V2 if the maintainers deem it's a cleaner solution.
Hm? I don't think there's any need to stick to a particular coding style. This file hasn't been touched a lot since 2010. Things moved on and replacing the gotos with straight return is totally fine.
(It has to pass the build checkers of course. However I don't think this will be a problem here...)
Cheers, Christian
| |