Re: [PATCH 31/31] ASoC: SOF: Use auto-cleanup for firmware loading

On Thu, 06 Aug 2026 15:04:33 +0200,
Péter Ujfalusi wrote:
> 
> 
> 
> On 06/08/2026 15:06, Takashi Iwai wrote:
> >>> -	fw_filename = kasprintf(GFP_KERNEL, "%s/%s", profile->fw_path,
> >>> -				profile->fw_name);
> >>> +	const char *fw_filename __free(kfree) =
> >>> +		kasprintf(GFP_KERNEL, "%s/%s", profile->fw_path,
> >>> +			  profile->fw_name);
> >>>  	if (!fw_filename)
> >>>  		return -ENOMEM;
> >>>  
> >>> +	const struct firmware *fw __free(firmware) = NULL;
> >>
> >> can we keep this at the head of the function?
> > 
> > We can, but for the auto-cleaned stuff, it's rather recommended to put
> > the declaration at the position that actually starts using it.  Then
> > you can avoid the unnecessary handling of the auto-cleanup.
> 
> The only mention I have seen is the header and it does say that one
> should avoid having the declaration and allocation split _if_ the
> pointer is not initialzied, or do it in one line.
> 
> Where it will get a bit blurry is when you have
> {
> const struct firmware *fw __free(firmware) = NULL;
> 
> ...
> ret = something_which_might_fail();
> if (ret)
> 	goto out;
> 
> ret = request_firmware();
> }
> 
> out:
> ...

Yes, and we hits this in a couple patches in this series, and the
workaround is to declare at the beginning.
(One can say that moving the declaration *is* helpful to catch such a
corner case :)

> > (I don't find whether it's mentioned, though; the documentation about
> > the auto-cleanup isn't well organized yet, unfortunately...)
> 
> OK, let's leave it as it is, my ack and tested tag still stands ;)

Thanks!


Takashi