Re: [PATCH 3/3] ASoC: SDCA: functions: Fix confusing cleanup.h syntax
From: Charles Keepax
Date: Thu Dec 04 2025 - 06:48:05 EST
On Wed, Dec 03, 2025 at 05:12:40PM +0100, Krzysztof Kozlowski wrote:
> Initializing automatic __free variables to NULL without need (e.g.
> branches with different allocations), followed by actual allocation is
> in contrary to explicit coding rules guiding cleanup.h:
>
> "Given that the "__free(...) = NULL" pattern for variables defined at
> the top of the function poses this potential interdependency problem the
> recommendation is to always define and assign variables in one statement
> and not group variable definitions at the top of the function when
> __free() is used."
>
> Code does not have a bug, but is less readable and uses discouraged
> coding practice, so fix that by moving declaration to the place of
> assignment.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxxxxx>
> ---
Hmm... yeah a fair point. Can't say I love the inline
declarations but I guess I will get used to it.
I wonder if for consistency we should do something about
the __free in find_sdca_init_table as well. One could move
the alloc to before the error checks, does risk doing an
unecessary alloc but its on the error path so I don't feel like
performance matters. Or rather than sizeof(*raw) we could just
do sizeof(struct raw_init_write)?
But if you would rather leave as is I don't mind either so:
Reviewed-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
Thanks,
Charles