netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: David Ahern <dsahern@gmail.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH 3/7] netlink: set extack error message in nla_validate()
Date: Wed, 19 Sep 2018 18:31:11 +0200	[thread overview]
Message-ID: <1537374671.10305.41.camel@sipsolutions.net> (raw)
In-Reply-To: <3e7aadff-5228-660a-537a-9b54c11fa5cc@gmail.com> (sfid-20180919_182023_809529_1EF8ED3D)

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

  reply	other threads:[~2018-09-19 22:10 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-19 12:08 [PATCH 0/7] netlink recursive policy validation Johannes Berg
2018-09-19 12:08 ` [PATCH 1/7] netlink: remove NLA_NESTED_COMPAT Johannes Berg
2018-09-19 12:08 ` [PATCH 2/7] netlink: make validation_data const Johannes Berg
2018-09-19 16:21   ` David Ahern
2018-09-19 12:08 ` [PATCH 3/7] netlink: set extack error message in nla_validate() Johannes Berg
2018-09-19 16:20   ` David Ahern
2018-09-19 16:31     ` Johannes Berg [this message]
2018-09-19 12:08 ` [PATCH 4/7] netlink: combine validate/parse functions Johannes Berg
2018-09-19 12:08 ` [PATCH 5/7] netlink: prepare validate extack setting for recursion Johannes Berg
2018-09-19 16:28   ` David Ahern
2018-09-19 16:36     ` Johannes Berg
     [not found]       ` <1537374995.10305.47.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
2018-09-19 16:44         ` David Ahern
2018-09-19 19:08           ` Marcelo Ricardo Leitner
2018-09-19 19:09             ` Johannes Berg
2018-09-19 12:08 ` [PATCH 6/7] netlink: allow NLA_NESTED to specify nested policy to validate Johannes Berg
2018-09-19 12:09 ` [PATCH 7/7] netlink: add nested array policy validation Johannes Berg
2018-09-19 12:15 ` [PATCH 0/7] netlink recursive " Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1537374671.10305.41.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=dsahern@gmail.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).