Re: [PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized

[Derek J. Clark]

On April 26, 2026 10:43:32 AM PDT, Yufei CHENG <cd345al@xxxxxxxxx> wrote:
>Hi Rong,
>
>Thanks for your feedback, I've corrected the format and resend the patch.
>However I think I have to point out some other issues in this file.
>
>First, we should not complexify simple things. You use lwmi_om_fan_get_set() to
>combine get and set into a single function with a boolean flag to determine the
>action.
>This makes the control flow harder to follow and audit. Furthermore, this layer
>of abstraction is not applied constantly throughout the whole file. In some
>places, values are set through lwmi_om_fan_get_set() and in other places,
>they're called manually through lwmi_dev_evaluate_int()
>This inconsistency makes the abstraction pointless and is the direct cause of a
>second uninitialized bug
>
>In line 855, args.arg0 is set to attribute_id, but arg1 is never set. Though
>arg1 is omitted in read action, you're still passing stack garbage to the
>lwmi_dev_evaluate_int(). And I think it's better to zero it out if not used.

Hi Yufei,

This bug is already addressed in a patch series that has been submitted and is awaiting review from the subsystem maintainers. 

>Additionally, I doubt whether bypassing the minimum RPM divisor has any
>practical use at all, not to mention it's not guranteed to work and purely
>depends on whether EC accepts that sub-division value.
>
>I may do some refactor recently, and I'm happy to hear your suggestions.

Rong has a large refactor series queued up to simplify much of the driver already, pending the acceptance of the previously mentioned series. It would be best to avoid doing any additional large refactoring until that series has landed.

Thanks,
Derek

>Thanks,
>Yufei
>