Re: [PATCH] virt/tdx: Enhance tdx-guest driver with improved memory management

[Dave Hansen <dave.hansen@intel.com>]

On 3/9/25 07:04, liu.yun@linux.dev wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> This patch refines the tdx-guest driver by introducing better memory
> management and error handling practices. The use of the `__free` attribute
> ensures that allocated memory for `reportdata` and `tdreport` is
> automatically freed, reducing the risk of memory leaks. Additionally,
> the manual mutex lock/unlock has been replaced with `scoped_cond_guard`
> to ensure proper mutex handling and simplify the code.
> 
> Error handling has been streamlined by returning directly on failure,
> eliminating unnecessary `goto` statements. These changes not only
> enhance the robustness of the driver but also improve its readability
> and maintainability.

Thanks for the patch.

But, no, sorry, we're not going to take patches like this. If you're
refactoring the code for _other_ reasons and want to convert over to the
new fancy stuff, go ahead.

But, we're not going to introduce bugs (and this kind of rework *WILL*
have bugs), make everyone else's code harder to merge, and clutter up
the history just to move to the newest shiny thing.

I'd much rather folks spend their time reviewing code or fixing bugs
than just churning code around.

Also, if anyone _does_ make code to use these new locks, *PLEASE* don't
do it this way:

> +	scoped_cond_guard(mutex_intr, return -EINTR, &quote_lock) {
> +		int ret;
> +		u8 *buf;
> +		struct tdx_quote_buf *quote_buf = quote_data;
> +		struct tsm_desc *desc = &report->desc;
> +		u64 err;

Indentation matters. Increasing the indenting on the whole function
makes it less readable. Don't do it like that ^.

I feel the need to reiterate: please don't send patches like this.
Please tell your friends.