Re: [PATCH] ASoC: cs-amp-lib: Use __free(kfree) instead of manual freeing

[Krzysztof Kozlowski <krzk@kernel.org>]

On 01/12/2025 12:30, Richard Fitzgerald wrote:
> On 01/12/2025 9:57 am, Richard Fitzgerald wrote:
>> On 29/11/2025 2:28 pm, Krzysztof Kozlowski wrote:
>>> On 27/11/2025 16:58, Richard Fitzgerald wrote:
>>>> Use the __free(kfree) cleanup to replace instances of manually
>>>> calling kfree(). Also make some code path simplifications that this
>>>> allows.
>>>>
>>>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>>>> ---
>>>>   sound/soc/codecs/cs-amp-lib.c | 29 ++++++++++++-----------------
>>>>   1 file changed, 12 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/sound/soc/codecs/cs-amp-lib.c b/sound/soc/codecs/cs-amp- 
>>>> lib.c
>>>> index d8f8b0259cd1..8c9fd9980a7d 100644
>>>> --- a/sound/soc/codecs/cs-amp-lib.c
>>>> +++ b/sound/soc/codecs/cs-amp-lib.c
>>>> @@ -7,6 +7,7 @@
>>>>   #include <asm/byteorder.h>
>>>>   #include <kunit/static_stub.h>
>>>> +#include <linux/cleanup.h>
>>>>   #include <linux/debugfs.h>
>>>>   #include <linux/dev_printk.h>
>>>>   #include <linux/efi.h>
>>>> @@ -309,9 +310,8 @@ static struct cirrus_amp_efi_data 
>>>> *cs_amp_get_cal_efi_buffer(struct device *dev,
>>>>                                    efi_guid_t **guid,
>>>>                                    u32 *attr)
>>>>   {
>>>> -    struct cirrus_amp_efi_data *efi_data;
>>>> +    struct cirrus_amp_efi_data *efi_data __free(kfree) = NULL;
>>>
>>> This is an undesired syntax explicitly documented as one to avoid. You
>>> need here proper assignment, not NULL. Please don't use cleanup.h if you
>>> do not intend to follow it because it does not make the code simpler.
>>>
>>
>> LOL
>> The new system to improve cleanup introduces new cleanup bugs. :)
>>
>>>
>>> Best regards,
>>> Krzysztof
>>
> I found 119 other instances of this _free(kfree) something = NULL; idiom
> in sound/ and ~300 across the whole kernel. So you've got quite some
> code to fix.

In few cases, when allocation is within some if() block, this is a
correct approach but most likely 90% of these 300 are same cases of not
following cleanup.h. And then also mixing it up with gotos (another
explicitly documented rule which people ignore).

Best regards,
Krzysztof