linux-kernel - Re: [PATCH] cxl/port: automate cleanup with _

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] [day] [month] [year] [list]

Message-ID: <2025062343-brilliant-laziness-e2cf@gregkh>
Date: Mon, 23 Jun 2025 11:27:33 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Pranav Tyagi <pranav.tyagi03@...il.com>
Cc: dave@...olabs.net, jonathan.cameron@...wei.com, dave.jiang@...el.com,
	alison.schofield@...el.com, vishal.l.verma@...el.com,
	ira.weiny@...el.com, dan.j.williams@...el.com, ming.li@...omail.com,
	rrichter@....com, jeff.johnson@....qualcomm.com,
	peterz@...radead.org, linux-cxl@...r.kernel.org,
	linux-kernel@...r.kernel.org, skhan@...uxfoundation.org,
	linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH] cxl/port: automate cleanup with __free()

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@...il.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