Mar 29, 2023 14:00:22 Mark Pearson <mpearson-lenovo@xxxxxxxxx>:
> Thanks Mirsad
>
> On Wed, Mar 29, 2023, at 2:49 PM, Mirsad Goran Todorovac wrote:
> <snip>
>>
>> Here is the patch proposal according to what Mark advised (using
>> different name for optitem):
>>
>> diff --git a/drivers/platform/x86/think-lmi.c
>> b/drivers/platform/x86/think-lmi.c
>> index c816646eb661..ab17254781c4 100644
>> --- a/drivers/platform/x86/think-lmi.c
>> +++ b/drivers/platform/x86/think-lmi.c
>> @@ -929,8 +929,10 @@ static ssize_t current_value_show(struct kobject
>> *kobj, struct kobj_attribute *a
>>
>> /* validate and split from `item,value` -> `value` */
>> value = strpbrk(item, ",");
>> - if (!value || value == item || !strlen(value + 1))
>> + if (!value || value == item || !strlen(value + 1)) {
>> + kfree(item);
>> return -EINVAL;
>> + }
>>
>> ret = sysfs_emit(buf, "%s\n", value + 1);
>> kfree(item);
>> @@ -1380,7 +1382,6 @@ static struct tlmi_pwd_setting
>> *tlmi_create_auth(const char *pwd_type,
>>
>> static int tlmi_analyze(void)
>> {
>> - acpi_status status;
>> int i, ret;
>>
>> if (wmi_has_guid(LENOVO_SET_BIOS_SETTINGS_GUID) &&
>> @@ -1417,8 +1418,8 @@ static int tlmi_analyze(void)
>> char *p;
>>
>> tlmi_priv.setting[i] = NULL;
>> - status = tlmi_setting(i, &item, LENOVO_BIOS_SETTING_GUID);
>> - if (ACPI_FAILURE(status))
>> + ret = tlmi_setting(i, &item, LENOVO_BIOS_SETTING_GUID);
>> + if (ret)
>
> Really minor, but tweak to be this and save a line of code?
This hunk is actually from another commit and should not be needed here.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/platform/x86/think-lmi.c?id=da62908efe80f132f691efc2ace4ca67626de86b
> + if (tlmi_setting(i, &item, LENOVO_BIOS_SETTING_GUID))
>
>> break;
>> if (!item)
>> break;
>> @@ -1457,10 +1458,10 @@ static int tlmi_analyze(void)
>> * name string.
>> * Try and pull that out if it's available.
>> */
>> - char *item, *optstart, *optend;
>> + char *optitem, *optstart, *optend;
>>
>> - if (!tlmi_setting(setting->index, &item,
>> LENOVO_BIOS_SETTING_GUID)) {
>> - optstart = strstr(item, "[Optional:");
>> + if (!tlmi_setting(setting->index, &optitem,
>> LENOVO_BIOS_SETTING_GUID)) {
>> + optstart = strstr(optitem,
>> "[Optional:");
>> if (optstart) {
>> optstart +=
>> strlen("[Optional:");
>> optend = strstr(optstart, "]");
>> @@ -1469,6 +1470,7 @@ static int tlmi_analyze(void)
>>
>> kstrndup(optstart, optend - optstart,
>>
>> GFP_KERNEL);
>> }
>> + kfree(optitem);
>> }
>> }
>> /*
>>
>> I have tested it, but without a few blunders of my own.
>
> I'm running a build locally and will aim to put this thru a few different platforms myself too and sanity check
>
>> I guess "nobody wins them all".
>
> I hear you :P
> > Mark
