Messages in this thread | | | From | <> | Subject | Re: [PATCH net-next] net: dcb: move getapptrust to separate function | Date | Mon, 14 Nov 2022 08:45:42 +0000 |
| |
> > > > + > > > > + 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. > > yup. > > And style trivia: > > I suggest adding error types after specific errors, > reversing the test and unindenting the code too. > > Something like: > > err = ops->dcbnl_getapptrust(netdev, selectors, &nselectors); > if (err) { > err = 0; > goto out; > } > > apptrust = nla_nest_start(skb, DCB_ATTR_DCB_APP_TRUST_TABLE); > if (!apptrust) { > err = -EMSGSIZE; > goto out; > } > > for (i = 0; i < nselectors; i++) { > enum ieee_attrs_app type = dcbnl_app_attr_type_get(selectors[i]); > err = nla_put_u8(skb, type, selectors[i]); > if (err) { > nla_nest_cancel(skb, apptrust); > goto out; > } > } > nla_nest_end(skb, apptrust); > > err = 0; > > out: > kfree(selectors); > return err; > } >
LGTM.
The last err = 0 can even be removed, as I see it. Will submit a new version with the changes suggested.
/ Daniel
| |