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

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

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>
> ---
>   tools/lib/thermal/commands.c | 41 ++++++++++++++++++++++++++++--------
>   1 file changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/lib/thermal/commands.c b/tools/lib/thermal/commands.c
> index 73d4d4e8d6ec..a9223df91dcf 100644
> --- a/tools/lib/thermal/commands.c
> +++ b/tools/lib/thermal/commands.c
> @@ -261,8 +261,23 @@ static struct genl_ops thermal_cmd_ops = {
>   	.o_ncmds	= ARRAY_SIZE(thermal_cmds),
>   };
>   
> -static thermal_error_t thermal_genl_auto(struct thermal_handler *th, int id, int cmd,
> -					 int flags, void *arg)
> +struct cmd_param {
> +	int tz_id;
> +};
> +
> +typedef int (*cmd_cb_t)(struct nl_msg *, struct cmd_param *);
> +
> +static int thermal_genl_tz_id_encode(struct nl_msg *msg, struct cmd_param *p)
> +{
> +	if (p->tz_id >= 0 && nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_ID, p->tz_id))
> +		return -1;
> +
> +	return 0;
> +}
> +
> +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?


The rest design looks good