On Mon, 05 Oct 2020 21:21:36 +0200 Johannes Berg wrote:
> > > But with the difference it seems to me that it'd be possible to get this
> > > mixed up?
> >
> > Right, I prefer not to have the unnecessary NLA_REJECTS, so my thinking
> > was - use the format I like for the new code, but leave the existing
> > rejects for a separate series / discussion.
> >
> > If we remove the rejects we still need something like
> >
> > extern struct nla_policy policy[lastattr + 1];
>
> Not sure I understand? You're using strict validation (I think), so
> attrs that are out of range will be rejected same as NLA_REJECT (well,
> with a different message) in __nla_validate_parse():
>
> nla_for_each_attr(nla, head, len, rem) {
> u16 type = nla_type(nla);
>
> if (type == 0 || type > maxtype) {
> if (validate & NL_VALIDATE_MAXTYPE) {
> NL_SET_ERR_MSG_ATTR(extack, nla,
> "Unknown attribute type");
> return -EINVAL;
> }
>
>
> In fact, if you're using strict validation even the default
> (0==NLA_UNSPEC) will be rejected, just like NLA_REJECT.
>
>
> Or am I confused somewhere?
Yea, I think we're both confused. Agreed with the above.
Are you suggesting:
const struct nla_policy policy[/* no size */] = {
[HEADER] = NLA_POLICY(...)
[OTHER_ATTR] = NLA_POLICY(...)
};
extern const struct nla_policy policy[/* no size */];
op = {
.policy = policy,
.max_attr = OTHER_ATTR,
}
?
What I'm saying is that my preference would be:
const struct nla_policy policy[OTHER_ATTR + 1] = {
[HEADER] = NLA_POLICY(...)
[OTHER_ATTR] = NLA_POLICY(...)
};
extern const struct nla_policy policy[OTHER_ATTR + 1];
op = {
.policy = policy,
.max_attr = ARRAY_SIZE(policy) - 1,
}
Since it's harder to forget to update the op (you don't have to update
op, and compiler will complain about the extern out of sync).
