Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Mixing goto-based error handling with scope-based cleanup helpers (`__free` and `guard`) in the same function. --
--- Patch [4]: [PATCH v10 4/4] firmware: coreboot: Add CFR firmware attributes driver --- commit f075e88024f4dbd198791a1dc20f657a026b2206 Author: Sean Rhodes <[email protected]> firmware: coreboot: Add CFR firmware attributes driver This commit adds a firmware-attributes driver for coreboot CFR settings. It exposes runtime EFI variable-backed configuration options to userspace via sysfs, allowing modification and handling of runtime-apply mechanisms. Link: https://doc.coreboot.org/drivers/cfr.html Link: https://doc.coreboot.org/drivers/cfr_internal.html > diff --git a/drivers/firmware/coreboot/coreboot-cfr.c > b/drivers/firmware/coreboot/coreboot-cfr.c > new file mode 100644 > index 0000000000000..21b06aa363111 > --- /dev/null > +++ b/drivers/firmware/coreboot/coreboot-cfr.c [ ... ] > +static int coreboot_cfr_write_value(struct coreboot_cfr_setting *setting, > + u32 value) > +{ > + efi_char16_t *efi_name __free(kfree) = NULL; > + u32 attrs; > + u32 old; > + int restore_ret; > + int ret; > + > + if (setting->read_only) > + return -EACCES; > + > + efi_name = coreboot_cfr_efi_name(setting->name); > + if (IS_ERR(efi_name)) > + return PTR_ERR(no_free_ptr(efi_name)); > + > + guard(mutex)(&setting->drvdata->lock); > + > + ret = efivar_lock(); > + if (ret) > + return ret; > + > + ret = coreboot_cfr_read_efi_value(efi_name, &old, &attrs); > + if (ret) > + goto out_unlock_efi; [ ... ] > + > +out_unlock_efi: > + efivar_unlock(); > + return ret; > +} [Severity: Medium] Does this mix goto-based error handling with scope-based cleanups in the same function? According to include/linux/cleanup.h, the expectation is that usage of goto and cleanup helpers (like __free and guard) should never be mixed in the same routine. Mixing them creates confusing resource ownership semantics and increases the risk of future resource leaks. Could this be unified to exclusively use either traditional goto labels or scope-based cleanups? For instance, efivar_lock() could potentially use a cleanup helper to avoid the goto out_unlock_efi labels entirely. [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
