On 5/21/26 09:14, Richard Fitzgerald wrote:
>>   -    sub = acpi_get_subsystem_id(ACPI_HANDLE(cs35l56->base.dev));
>> +    const char *sub __free(kfree) = acpi_get_subsystem_id(ACPI_HANDLE(cs35l56->base.dev));
>> +    const char *system_name __free(kfree) = NULL;
>> +
> 
> Initializing a __free(kfree) to NULL is controversial. The kerneldoc
> warns against it. Though I think your code is totally fine it might be
> best to avoid.

I used __free(kfree) for the derived system_name to keep
ownership transfer local until read_acpi() had passed the reset GPIO error
path, but I agree that initializing a cleanup variable to NULL is best
avoided.

> 
> I can't see any reason why we didn't use devm cleanup for
> cs35l56->system_name.
> 
> So
> 
>            if (ret == -ENOENT) {
>             cs35l56->system_name = devm_kstrdup(dev, sub,
> GFP_KERNEL);
>           } else if (ret >= 0) {
>             cs35l56->system_name = devm_kasprintf(dev, ...);
>             ...
> 
> 
> and delete the manual kfree(cs35l56->system_name) in
> cs35l56_hda_remove(). 

I'll send a v3 with that approach.


Thanks,
Cássio