Re: [PATCH] cxl/port: automate cleanup with __free()

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jun 23, 2025 at 02:49:29PM +0530, Pranav Tyagi wrote:
> Use the scope based resource management (defined in linux/cleanup.h) to
> automate the lifetime control of struct cxl_endpoint_decoder. This
> eliminates the explicit kfree() call and makes the code more robust and
> maintainable in presence of early returns.
> 
> Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
> ---
>  drivers/cxl/core/port.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index eb46c6764d20..c35946882b20 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -10,6 +10,7 @@
>  #include <linux/slab.h>
>  #include <linux/idr.h>
>  #include <linux/node.h>
> +#include <linux/cleanup.h>
>  #include <cxl/einj.h>
>  #include <cxlmem.h>
>  #include <cxlpci.h>
> @@ -1888,14 +1889,14 @@ EXPORT_SYMBOL_NS_GPL(cxl_switch_decoder_alloc, "CXL");
>   */
>  struct cxl_endpoint_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port)
>  {
> -	struct cxl_endpoint_decoder *cxled;
>  	struct cxl_decoder *cxld;
>  	int rc;
>  
>  	if (!is_cxl_endpoint(port))
>  		return ERR_PTR(-EINVAL);
>  
> -	cxled = kzalloc(sizeof(*cxled), GFP_KERNEL);
> +	struct cxl_endpoint_decoder *cxled __free(kfree) =
> +		kzalloc(sizeof(*cxled), GFP_KERNEL);
>  	if (!cxled)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -1904,7 +1905,6 @@ struct cxl_endpoint_decoder *cxl_endpoint_decoder_alloc(struct cxl_port *port)
>  	cxld = &cxled->cxld;
>  	rc = cxl_decoder_init(port, cxld);
>  	if (rc)	 {
> -		kfree(cxled);
>  		return ERR_PTR(rc);
>  	}
>  
> -- 
> 2.49.0

Note, I can't speak for the maintainers of this subsystem, but
generally, making changes like this for no real good reason, for code
that has been around for years, is really not needed at all.

If you fix a bug with it, sure, but changes for the sake of "let's use
this new feature" in here really might not be necessary.

Why not add cleanup.h support to code paths that actually fix existing
bugs instead?

Also, you have added some coding style errors to the code now with this
patch, which also is generally not considered a good idea :)

thanks,

greg k-h