Re: [PATCH libnftnl 3/3] desc: add set description

[Phil Sutter <phil@nwl.cc>]

Hi,

On Thu, Jan 20, 2022 at 01:04:02AM +0100, Pablo Neira Ayuso wrote:
[...]
> diff --git a/src/desc.c b/src/desc.c
> index f73e74c2c7d3..c8b3195db850 100644
> --- a/src/desc.c
> +++ b/src/desc.c
[...]
> +static int nftnl_set_desc_build_dtype(struct nftnl_udata_buf *udbuf,
> +				      const struct nftnl_set_desc *dset)
> +{
> +	struct nftnl_udata *nest;
> +	int i, err;
> +
> +	switch (dset->type) {
> +	case NFTNL_DESC_SET_TYPEOF:
> +		nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_KEY);
> +		for (i = 0; i < dset->key.num_type; i++) {
> +			err = __nftnl_udata_set_dtype_build(udbuf, dset->key.dtype[i], i);
> +			if (err < 0)
> +				return err;
> +		}
> +		nftnl_udata_nest_end(udbuf, nest);
> +		break;
> +	case NFTNL_DESC_SET_DATATYPE:
> +		nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_DATA);
> +		for (i = 0; i < dset->data.num_type; i++) {
> +			err = __nftnl_udata_set_dtype_build(udbuf, dset->data.dtype[i], i);
> +			if (err < 0)
> +				return err;
> +		}
> +		nftnl_udata_nest_end(udbuf, nest);
> +		break;
> +	case NFTNL_DESC_SET_UNSPEC:
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int __nftnl_set_desc_build_typeof(struct nftnl_udata_buf *udbuf,
> +					 const struct nftnl_expr_desc *dexpr,
> +					 uint8_t attr_type)
> +{
> +	struct nftnl_udata *nest;
> +	int err;
> +
> +	nest = nftnl_udata_nest_start(udbuf, attr_type);
> +	err = nftnl_expr_desc_build(udbuf, dexpr);
> +	nftnl_udata_nest_end(udbuf, nest);
> +
> +	return err;
> +}
> +
> +static int nftnl_set_desc_build_typeof(struct nftnl_udata_buf *udbuf,
> +				       const struct nftnl_set_desc *dset)
> +{
> +	struct nftnl_udata *nest;
> +	int i;
> +
> +	switch (dset->type) {
> +	case NFTNL_DESC_SET_TYPEOF:
> +		nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_KEY_TYPEOF);
> +		for (i = 0; i < dset->key.num_typeof; i++)
> +			__nftnl_set_desc_build_typeof(udbuf, dset->key.expr[i], i);
> +
> +		nftnl_udata_nest_end(udbuf, nest);
> +		break;
> +	case NFTNL_DESC_SET_DATATYPE:
> +		nest = nftnl_udata_nest_start(udbuf, NFTNL_UDATA_SET_DATA_TYPEOF);
> +		for (i = 0; i < dset->key.num_typeof; i++)
> +			__nftnl_set_desc_build_typeof(udbuf, dset->data.expr[i], i);
> +
> +		nftnl_udata_nest_end(udbuf, nest);
> +		break;
> +	case NFTNL_DESC_SET_UNSPEC:
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +EXPORT_SYMBOL(nftnl_set_desc_build_udata);
> +int nftnl_set_desc_build_udata(struct nftnl_udata_buf *udbuf,
> +			       const struct nftnl_set_desc *dset)
> +{
> +	if (!nftnl_udata_put_u32(udbuf, NFTNL_UDATA_SET_FLAGS, dset->flags))
> +		return -1;
> +
> +	switch (dset->type) {
> +	case NFTNL_DESC_SET_DATATYPE:
> +		return nftnl_set_desc_build_dtype(udbuf, dset);
> +	case NFTNL_DESC_SET_TYPEOF:
> +		return nftnl_set_desc_build_typeof(udbuf, dset);
> +	case NFTNL_DESC_SET_UNSPEC:
> +		return -1;
> +	}
> +
> +	if (!nftnl_udata_put_strz(udbuf, NFTNL_UDATA_SET_COMMENT, dset->comment))
> +		return -1;
> +
> +	return -1;
> +}

This is odd: Depending on dset->type, nftnl_set_desc_build_udata() calls
either nftnl_set_desc_build_dtype() or nftnl_set_desc_build_typeof().
Yet both check dset->type again. This looks like a mix-up of set/map key
and data definitions and typeof vs. "regular" definition styles.

Cheers, Phil