sound/soc/codecs/cs-amp-lib.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-)
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;
unsigned long data_size = 0;
- u8 *data;
efi_status_t status;
int i, ret;
@@ -339,19 +339,18 @@ static struct cirrus_amp_efi_data *cs_amp_get_cal_efi_buffer(struct device *dev,
}
/* Get variable contents into buffer */
- data = kmalloc(data_size, GFP_KERNEL);
- if (!data)
+ efi_data = kmalloc(data_size, GFP_KERNEL);
+ if (!efi_data)
return ERR_PTR(-ENOMEM);
status = cs_amp_get_efi_variable(cs_amp_lib_cal_efivars[i].name,
cs_amp_lib_cal_efivars[i].guid,
- attr, &data_size, data);
+ attr, &data_size, efi_data);
if (status != EFI_SUCCESS) {
ret = -EINVAL;
goto err;
}
- efi_data = (struct cirrus_amp_efi_data *)data;
dev_dbg(dev, "Calibration: Size=%d, Amp Count=%d\n", efi_data->size, efi_data->count);
if ((efi_data->count > 128) ||
@@ -365,10 +364,9 @@ static struct cirrus_amp_efi_data *cs_amp_get_cal_efi_buffer(struct device *dev,
if (efi_data->size == 0)
efi_data->size = data_size;
- return efi_data;
+ return_ptr(efi_data);
err:
- kfree(data);
dev_err(dev, "Failed to read calibration data from EFI: %d\n", ret);
return ERR_PTR(ret);
@@ -391,9 +389,9 @@ static int cs_amp_set_cal_efi_buffer(struct device *dev,
static int _cs_amp_get_efi_calibration_data(struct device *dev, u64 target_uid, int amp_index,
struct cirrus_amp_cal_data *out_data)
{
- struct cirrus_amp_efi_data *efi_data;
+ struct cirrus_amp_efi_data *efi_data __free(kfree) = NULL;
struct cirrus_amp_cal_data *cal = NULL;
- int i, ret;
+ int i;
efi_data = cs_amp_get_cal_efi_buffer(dev, NULL, NULL, NULL);
if (IS_ERR(efi_data))
@@ -434,17 +432,14 @@ static int _cs_amp_get_efi_calibration_data(struct device *dev, u64 target_uid,
dev_warn(dev, "Calibration entry %d does not match silicon ID", amp_index);
}
- if (cal) {
- memcpy(out_data, cal, sizeof(*out_data));
- ret = 0;
- } else {
+ if (!cal) {
dev_warn(dev, "No calibration for silicon ID %#llx\n", target_uid);
- ret = -ENOENT;
+ return -ENOENT;
}
- kfree(efi_data);
+ memcpy(out_data, cal, sizeof(*out_data));
- return ret;
+ return 0;
}
static int _cs_amp_set_efi_calibration_data(struct device *dev, int amp_index, int num_amps,
--
2.47.3On 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.
Best regards,
Krzysztof
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
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.
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
On Thu, 27 Nov 2025 15:58:17 +0000, Richard Fitzgerald wrote:
> Use the __free(kfree) cleanup to replace instances of manually
> calling kfree(). Also make some code path simplifications that this
> allows.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: cs-amp-lib: Use __free(kfree) instead of manual freeing
commit: 6797540c8b76dd847466b9a8d6e635e6a2ac95d3
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
© 2016 - 2025 Red Hat, Inc.