Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ASoC: cs-amp-lib: Revert use of __free(kfree) back to normal C cleanup

Revert commit 6797540c8b76 ("ASoC: cs-amp-lib: Use __free(kfree) instead
of manual freeing").

Krzysztof Kozlowski pointed out that __free() can be dangerous.
It can introduce new cleanup bugs. These are more subtle and difficult to
spot than a missing goto in traditional cleanup, because they are triggered
by writing regular idiomatic C code instead of using C++ conventions. As
it's regular C style it's more likely to be missed because the code is as
would be expected for C. The traditional goto also more obviously flags
to anyone changing the code in the future that they must be careful about
the cleanup.

We can just revert the change. There was nothing wrong with the original
code and as Krzysztof noted: "it does not make the code simpler."

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 6797540c8b76 ("ASoC: cs-amp-lib: Use __free(kfree) instead of manual freeing")
Link: https://patch.msgid.link/20251201111429.43517-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>

authored by

Richard Fitzgerald and committed by
Mark Brown
2b69bee5 c5fae31f

+17 -12
+17 -12
sound/soc/codecs/cs-amp-lib.c
··· 7 7 8 8 #include <asm/byteorder.h> 9 9 #include <kunit/static_stub.h> 10 - #include <linux/cleanup.h> 11 10 #include <linux/debugfs.h> 12 11 #include <linux/dev_printk.h> 13 12 #include <linux/efi.h> ··· 309 310 efi_guid_t **guid, 310 311 u32 *attr) 311 312 { 312 - struct cirrus_amp_efi_data *efi_data __free(kfree) = NULL; 313 + struct cirrus_amp_efi_data *efi_data; 313 314 unsigned long data_size = 0; 315 + u8 *data; 314 316 efi_status_t status; 315 317 int i, ret; 316 318 ··· 339 339 } 340 340 341 341 /* Get variable contents into buffer */ 342 - efi_data = kmalloc(data_size, GFP_KERNEL); 343 - if (!efi_data) 342 + data = kmalloc(data_size, GFP_KERNEL); 343 + if (!data) 344 344 return ERR_PTR(-ENOMEM); 345 345 346 346 status = cs_amp_get_efi_variable(cs_amp_lib_cal_efivars[i].name, 347 347 cs_amp_lib_cal_efivars[i].guid, 348 - attr, &data_size, efi_data); 348 + attr, &data_size, data); 349 349 if (status != EFI_SUCCESS) { 350 350 ret = -EINVAL; 351 351 goto err; 352 352 } 353 353 354 + efi_data = (struct cirrus_amp_efi_data *)data; 354 355 dev_dbg(dev, "Calibration: Size=%d, Amp Count=%d\n", efi_data->size, efi_data->count); 355 356 356 357 if ((efi_data->count > 128) || ··· 365 364 if (efi_data->size == 0) 366 365 efi_data->size = data_size; 367 366 368 - return_ptr(efi_data); 367 + return efi_data; 369 368 370 369 err: 370 + kfree(data); 371 371 dev_err(dev, "Failed to read calibration data from EFI: %d\n", ret); 372 372 373 373 return ERR_PTR(ret); ··· 391 389 static int _cs_amp_get_efi_calibration_data(struct device *dev, u64 target_uid, int amp_index, 392 390 struct cirrus_amp_cal_data *out_data) 393 391 { 394 - struct cirrus_amp_efi_data *efi_data __free(kfree) = NULL; 392 + struct cirrus_amp_efi_data *efi_data; 395 393 struct cirrus_amp_cal_data *cal = NULL; 396 - int i; 394 + int i, ret; 397 395 398 396 efi_data = cs_amp_get_cal_efi_buffer(dev, NULL, NULL, NULL); 399 397 if (IS_ERR(efi_data)) ··· 434 432 dev_warn(dev, "Calibration entry %d does not match silicon ID", amp_index); 435 433 } 436 434 437 - if (!cal) { 435 + if (cal) { 436 + memcpy(out_data, cal, sizeof(*out_data)); 437 + ret = 0; 438 + } else { 438 439 dev_warn(dev, "No calibration for silicon ID %#llx\n", target_uid); 439 - return -ENOENT; 440 + ret = -ENOENT; 440 441 } 441 442 442 - memcpy(out_data, cal, sizeof(*out_data)); 443 + kfree(efi_data); 443 444 444 - return 0; 445 + return ret; 445 446 } 446 447 447 448 static int _cs_amp_set_efi_calibration_data(struct device *dev, int amp_index, int num_amps,