Subject: Re: [PATCH net-next] net: dcb: move getapptrust to separate function

From: Petr Machata <[email-masked]>
Date: Thu, 10 Nov 2022 17:30:43 +0100

Daniel Machon <[email-masked]> writes:

> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index cec0632f96db..3f4d88c1ec78 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -1060,11 +1060,52 @@ static int dcbnl_build_peer_app(struct net_device *netdev, struct sk_buff* skb,
>  	return err;
>  }
>  
> +static int dcbnl_getapptrust(struct net_device *netdev, struct sk_buff *skb)
> +{
> +	const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
> +	int nselectors, err;
> +	u8 *selectors;
> +
> +	selectors = kzalloc(IEEE_8021QAZ_APP_SEL_MAX + 1, GFP_KERNEL);
> +	if (!selectors)
> +		return -ENOMEM;
> +
> +	err = ops->dcbnl_getapptrust(netdev, selectors, &nselectors);
> +
> +	if (!err) {
> +		struct nlattr *apptrust;
> +		int i;

(Maybe consider moving these up to the function scope. This scope
business made sense in the generic function, IMHO is not as useful with
a focused function like this one.)

> +
> +		err = -EMSGSIZE;
> +
> +		apptrust = nla_nest_start(skb, DCB_ATTR_DCB_APP_TRUST_TABLE);
> +		if (!apptrust)
> +			goto nla_put_failure;
> +
> +		for (i = 0; i < nselectors; i++) {
> +			enum ieee_attrs_app type =
> +				dcbnl_app_attr_type_get(selectors[i]);

Doesn't checkpatch warn about this? There should be a blank line after
the variable declaration block. (Even if there wasn't one there in the
original code either.)

> +			err = nla_put_u8(skb, type, selectors[i]);
> +			if (err) {
> +				nla_nest_cancel(skb, apptrust);
> +				goto nla_put_failure;
> +			}
> +		}
> +		nla_nest_end(skb, apptrust);
> +	}
> +
> +	err = 0;
> +
> +nla_put_failure:
> +	kfree(selectors);
> +	return err;
> +}
> +