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

Lukasz Luba
2024-10-21T19:48:39Z

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