Re: [PATCH] soc: aspeed: deregister a misc device on error

[Andrew Jeffery <andrew@codeconstruct.com.au>]

Hi Joe,

On Sat, 2024-12-07 at 12:25 +0900, Joe Hattori wrote:
> The error path of aspeed_lpc_enable_snoop() does not deregister the
> misc
> device, which results in a memory leak. Therefore, add a
> misc_deregister() call in the error path.
> 
> Fixes: 524feb799408 ("soc: add aspeed folder and misc drivers")
> Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp>
> ---
>  drivers/soc/aspeed/aspeed-lpc-snoop.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/soc/aspeed/aspeed-lpc-snoop.c
> b/drivers/soc/aspeed/aspeed-lpc-snoop.c
> index 9ab5ba9cf1d6..083ddf6dcb7a 100644
> --- a/drivers/soc/aspeed/aspeed-lpc-snoop.c
> +++ b/drivers/soc/aspeed/aspeed-lpc-snoop.c
> @@ -221,6 +221,7 @@ static int aspeed_lpc_enable_snoop(struct
> aspeed_lpc_snoop *lpc_snoop,
>                 hicrb_en = HICRB_ENSNP1D;
>                 break;
>         default:
> +               misc_deregister(&lpc_snoop->chan[channel].miscdev);

We should free the kfifo too.

Anyway, all the switch statement is doing is setting up mask metadata,
the non-default cases don't depend on the acquired resources. I think
it would make more sense to move it prior to any resource acquisition,
rather than try to unwind their acquisition in the default case.

Andrew