| Message ID | 20180919120900.28708-4-johannes@sipsolutions.net (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | netlink recursive policy validation | expand |
On 9/19/18 5:08 AM, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > In nla_parse() we already set this, but it makes sense to > also do it in nla_validate() which already also sets the > bad attribute pointer. > > CC: David Ahern <dsahern@gmail.com> > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > lib/nlattr.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/lib/nlattr.c b/lib/nlattr.c > index e2e5b38394d5..33404745bec4 100644 > --- a/lib/nlattr.c > +++ b/lib/nlattr.c > @@ -180,9 +180,13 @@ int nla_validate(const struct nlattr *head, int len, int maxtype, > int rem; > > nla_for_each_attr(nla, head, len, rem) { > - int err = validate_nla(nla, maxtype, policy, NULL); > + static const char _msg[] = "Attribute failed policy validation"; > + const char *msg = _msg; > + int err = validate_nla(nla, maxtype, policy, &msg); > > if (err < 0) { > + if (extack) > + extack->_msg = msg; > NL_SET_BAD_ATTR(extack, nla); msg, NL_SET_BAD_ATTR and extack handling all can be done in validate_nla removing the need for the same message ("Attribute failed policy validation") to be declared twice and simplifying the extack setting. > return err; > } >
On Wed, 2018-09-19 at 09:20 -0700, David Ahern wrote: > > nla_for_each_attr(nla, head, len, rem) { > > - int err = validate_nla(nla, maxtype, policy, NULL); > > + static const char _msg[] = "Attribute failed policy validation"; > > + const char *msg = _msg; > > + int err = validate_nla(nla, maxtype, policy, &msg); > > > > if (err < 0) { > > + if (extack) > > + extack->_msg = msg; > > NL_SET_BAD_ATTR(extack, nla); > > msg, NL_SET_BAD_ATTR and extack handling all can be done in validate_nla > removing the need for the same message ("Attribute failed policy > validation") to be declared twice and simplifying the extack setting. Yeah, perhaps I should take another look at that. I didn't want to do that originally as validate_nla() has so many exit points, but perhaps it's better to put a goto label there. FWIW, in the next patch I'm getting rid of the duplication again - I wanted to have the patch that has an effect (this one, setting the message) more clearly separated. johannes
diff --git a/lib/nlattr.c b/lib/nlattr.c index e2e5b38394d5..33404745bec4 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -180,9 +180,13 @@ int nla_validate(const struct nlattr *head, int len, int maxtype, int rem; nla_for_each_attr(nla, head, len, rem) { - int err = validate_nla(nla, maxtype, policy, NULL); + static const char _msg[] = "Attribute failed policy validation"; + const char *msg = _msg; + int err = validate_nla(nla, maxtype, policy, &msg); if (err < 0) { + if (extack) + extack->_msg = msg; NL_SET_BAD_ATTR(extack, nla); return err; }