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

[Lukasz Luba <lukasz.luba@arm.com>]

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...