Messages in this thread | | | From | Petr Machata <> | Subject | Re: [PATCH net-next] net: dcb: move getapptrust to separate function | Date | Fri, 11 Nov 2022 12:24:49 +0100 |
| |
<Daniel.Machon@microchip.com> writes:
> Den Thu, Nov 10, 2022 at 05:30:43PM +0100 skrev Petr Machata: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> Daniel Machon <daniel.machon@microchip.com> 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.) > > I dont mind doing that, however, this 'scope business' is just staying true > to the rest of the dcbnl code :-) - that said, I think I agree with your > point. > >> >> > + >> > + 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.) > > Nope, no warning. And I think it has something to do with the way the line > is split.
OK. I find the code readable just fine, so I'm fine with it as it stands:
Reviewed-by: Petr Machata <petrm@nvidia.com>
| |