Re: [LTP] [PATCH v3 1/4] tst_netlink: Add helper functions for handling generic attributes

Petr Vorel
2023-11-21T10:42:00Z

Hi Martin,

> Refactor struct tst_rtnl_attr_list for generic use and add helper
> functions for handling generic struct nlattr message attributes.

There is still tst_rtnl_attr_list mentioned at doc/C-Test-Network-API.asciidoc,
could you please update that?

...
> -/* Add arbitrary attribute to last message */
> +/* Add arbitrary nlattr attribute to last message */
> +int tst_netlink_add_attr(const char *file, const int lineno,
> + struct tst_netlink_context *ctx, unsigned short type, const void *data,
> + unsigned short len);
> +#define NETLINK_ADD_ATTR(ctx, type, data, len) \
> + tst_netlink_add_attr(__FILE__, __LINE__, (ctx), (type), (data), (len))
> +
> +/* Add string nlattr attribute to last message */
> +int tst_netlink_add_attr_string(const char *file, const int lineno,
> + struct tst_netlink_context *ctx, unsigned short type, const char *data);
> +#define NETLINK_ADD_ATTR_STRING(ctx, type, data) \
> + tst_netlink_add_attr_string(__FILE__, __LINE__, (ctx), (type), (data))

FYI NETLINK_ADD_ATTR_STRING() is not used anywhere. I suppose you plan to use it
in some of the following CVE tests, right?

...
> +int tst_netlink_add_attr_list(const char *file, const int lineno,
> + struct tst_netlink_context *ctx,
> + const struct tst_netlink_attr_list *list)
> +{
> + int i, ret;
> + size_t offset;
> +
> + for (i = 0; list[i].len >= 0; i++) {
> + if (list[i].len > USHRT_MAX) {
> + tst_brk_(file, lineno, TBROK,
> + "%s(): Attribute value too long", __func__);
> + return -1;
Here (and on other function working on more items) is return -1 on error and >=
0 on success. The other functions return 0 on error and 1 on success.
While this is logic, it would still be nice to describe return in tst_netlink.h.

The rest LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr