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

[Petr Vorel <pvorel@suse.cz>]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp