Re: [PATCH v5 2/4] tools/lib/thermal: Make more generic the command encoding function

Lukasz Luba
2024-10-22T09:42:16Z

On 10/22/24 08:12, Daniel Lezcano wrote:
> On 21/10/2024 21:49, Lukasz Luba wrote:
>>
>>
>> On 10/14/24 10:43, Daniel Lezcano wrote:
>>> The thermal netlink has been extended with more commands which require
>>> an encoding with more information. The generic encoding function puts
>>> the thermal zone id with the command name. It is the unique
>>> parameters.
>>>
>>> The next changes will provide more parameters to the command. Set the
>>> scene for those new parameters by making the encoding function more
>>> generic.
>>>
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> ---
>
> [ ... ]
>
>>> +static thermal_error_t thermal_genl_auto(struct thermal_handler *th,
>>> cmd_cb_t cmd_cb,
>>> +                     struct cmd_param *param,
>>> +                     int cmd, int flags, void *arg)
>>>   {
>>>       struct nl_msg *msg;
>>>       void *hdr;
>>> @@ -276,7 +291,7 @@ static thermal_error_t thermal_genl_auto(struct
>>> thermal_handler *th, int id, int
>>>       if (!hdr)
>>>           return THERMAL_ERROR;
>>> -    if (id >= 0 && nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, id))
>>> +    if (cmd_cb && cmd_cb(msg, param))
>>>           return THERMAL_ERROR;
>>
>> It's not in this code but also in older:
>> shouldn't we free the nlmsg_free(msg); before returns in this
>> function?
>
> Right, thanks for pointing this out
>
> If it is ok, I will send a patch on top of this series to fix the entire
> function
>
>

That fine for me, unless Rafael wants a v6 version with those
new locking. I'm OK with both, I will review any version.

BTW I'm still struggling to cross-build or native build that
libthermal, so I cannot give you test results.
I need to sort our the headers and OS packages on dev board...