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: <CAH4c4jLH2XoqfVOx-7JxTw-fX8Xbc2zhGmCh31e4v2gQc7xMoA@mail.gmail.com>
Date: Thu, 26 Jun 2025 19:58:33 +0530
From: Pranav Tyagi <pranav.tyagi03@...il.com>
To: Dan Williams <dan.j.williams@...el.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, 
	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 Tue, Jun 24, 2025 at 10:42 PM Dan Williams <dan.j.williams@...el.com> wrote:
>
> 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.
>
> I do not want to review small standalone conversions of individual
> functions for no other reason than vague claims of improvement. The
> reasons to convert an existing function to cleanup.h helpers are:
>
> 1/ to fix an actual bug
>
> 2/ in the course of refactoring the code for other reasons
>
> 3/ to eliminate goto trickiness and bulk
>
> This patch does not make the code "more robust and maintainable", it is
> just churn given how easy it is to verify that the kfree() is correctly
> paired.
>
> One quick way to identify code churn is when the diffstat does not
> result in a net reduction in code lines:
>
> This patch is "3 insertions(+), 3 deletions(-)" == "churn".

Thank you for the feedback. I understand your concerns and completely
agree with your reasoning. Please pardon my misjudgment in sending this
patch. I am still a beginner with kernel development and learning to
better assess what makes a meaningful contribution.

Regards
Pranav Tyagi