* [PATCH] cfg80211: introduce nl80211_get_ifidx()
@ 2009-11-10 11:51 Holger Schurig
2009-11-10 14:50 ` pat-lkml
0 siblings, 1 reply; 4+ messages in thread
From: Holger Schurig @ 2009-11-10 11:51 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
... which get's rid of three indentical cut-n-paste sections.
Signed-off-by: Holger Schurig <holgerschurig@gmail.com>
---
... and so I don't need to copy the same a fourth time when I change
get_survey() into dump_survey() :-)
--- linux-wl.orig/net/wireless/nl80211.c
+++ linux-wl/net/wireless/nl80211.c
@@ -151,6 +151,26 @@
[NL80211_KEY_DEFAULT_MGMT] = { .type = NLA_FLAG },
};
+/* ifidx get helper */
+static int nl80211_get_ifidx(struct netlink_callback *cb)
+{
+ int res;
+
+ res = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
+ nl80211_fam.attrbuf, nl80211_fam.maxattr,
+ nl80211_policy);
+ if (res)
+ return res;
+
+ if (!nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX])
+ return -EINVAL;
+
+ res = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX]);
+ if (!res)
+ return -EINVAL;
+ return res;
+}
+
/* IE validation */
static bool is_valid_ie_attr(const struct nlattr *attr)
{
@@ -1682,20 +1702,10 @@
int sta_idx = cb->args[1];
int err;
- if (!ifidx) {
- err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
- nl80211_fam.attrbuf, nl80211_fam.maxattr,
- nl80211_policy);
- if (err)
- return err;
-
- if (!nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX])
- return -EINVAL;
-
- ifidx = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX]);
- if (!ifidx)
- return -EINVAL;
- }
+ if (!ifidx)
+ ifidx = nl80211_get_ifidx(cb);
+ if (ifidx < 0)
+ return ifidx;
rtnl_lock();
@@ -2145,20 +2155,10 @@
int path_idx = cb->args[1];
int err;
- if (!ifidx) {
- err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
- nl80211_fam.attrbuf, nl80211_fam.maxattr,
- nl80211_policy);
- if (err)
- return err;
-
- if (!nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX])
- return -EINVAL;
-
- ifidx = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX]);
- if (!ifidx)
- return -EINVAL;
- }
+ if (!ifidx)
+ ifidx = nl80211_get_ifidx(cb);
+ if (ifidx < 0)
+ return ifidx;
rtnl_lock();
@@ -3182,20 +3182,11 @@
int err;
if (!ifidx) {
- err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
- nl80211_fam.attrbuf, nl80211_fam.maxattr,
- nl80211_policy);
- if (err)
- return err;
-
- if (!nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX])
- return -EINVAL;
-
- ifidx = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX]);
- if (!ifidx)
- return -EINVAL;
+ ifidx = nl80211_get_ifidx(cb);
cb->args[0] = ifidx;
}
+ if (ifidx < 0)
+ return ifidx;
dev = dev_get_by_index(sock_net(skb->sk), ifidx);
if (!dev)
--
http://www.holgerschurig.de
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] cfg80211: introduce nl80211_get_ifidx()
2009-11-10 11:51 [PATCH] cfg80211: introduce nl80211_get_ifidx() Holger Schurig
@ 2009-11-10 14:50 ` pat-lkml
2009-11-10 15:15 ` Holger Schurig
0 siblings, 1 reply; 4+ messages in thread
From: pat-lkml @ 2009-11-10 14:50 UTC (permalink / raw)
To: Holger Schurig; +Cc: linux-wireless, Johannes Berg
On Tue, 10 Nov 2009 12:51:43 +0100, Holger Schurig
<holgerschurig@googlemail.com> wrote:
> @@ -3182,20 +3182,11 @@
> int err;
>
> if (!ifidx) {
> - err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
> - nl80211_fam.attrbuf, nl80211_fam.maxattr,
> - nl80211_policy);
> - if (err)
> - return err;
> -
> - if (!nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX])
> - return -EINVAL;
> -
> - ifidx = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX]);
> - if (!ifidx)
> - return -EINVAL;
> + ifidx = nl80211_get_ifidx(cb);
do you need an:
if(ifidx < 0)
return ifidx;
here, as you assign it to cb->args[0], which differs from the original
behavior.
> cb->args[0] = ifidx;
> }
> + if (ifidx < 0)
> + return ifidx;
>
> dev = dev_get_by_index(sock_net(skb->sk), ifidx);
> if (!dev)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] cfg80211: introduce nl80211_get_ifidx()
2009-11-10 14:50 ` pat-lkml
@ 2009-11-10 15:15 ` Holger Schurig
2009-11-10 15:41 ` pat-lkml
0 siblings, 1 reply; 4+ messages in thread
From: Holger Schurig @ 2009-11-10 15:15 UTC (permalink / raw)
To: linux-wireless; +Cc: pat-lkml, Holger Schurig, Johannes Berg
On Tuesday 10 November 2009 15:50:43 pat-lkml@erley.org wrote:
> On Tue, 10 Nov 2009 12:51:43 +0100, Holger Schurig
> <holgerschurig@googlemail.com> wrote:
>
> > @@ -3182,20 +3182,11 @@
> > int err;
> >
> > if (!ifidx) {
> > - err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
> > - nl80211_fam.attrbuf, nl80211_fam.maxattr,
> > - nl80211_policy);
> > - if (err)
> > - return err;
> > -
> > - if (!nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX])
> > - return -EINVAL;
> > -
> > - ifidx = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX]);
> > - if (!ifidx)
> > - return -EINVAL;
> > + ifidx = nl80211_get_ifidx(cb);
>
> do you need an:
>
> if(ifidx < 0)
> return ifidx;
>
> here, as you assign it to cb->args[0], which differs from the original
> behavior.
Do you mean
if (!ifidx) {
ifidx = nl80211_get_ifidx(cb);
if (ifidx < 0)
return ifidx;
cb->args[0] = ifidx;
}
if (ifidx < 0)
return ifidx;
instead?
I'm not familiar with this function, but maybe we
can get away like this:
int ifidx = cb->args[0];
if (!ifidx)
ifidx = nl80211_get_ifidx(cb);
if (ifidx < 0)
return ifidx;
cb->args[0] = ifidx;
--
http://www.holgerschurig.de
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] cfg80211: introduce nl80211_get_ifidx()
2009-11-10 15:15 ` Holger Schurig
@ 2009-11-10 15:41 ` pat-lkml
0 siblings, 0 replies; 4+ messages in thread
From: pat-lkml @ 2009-11-10 15:41 UTC (permalink / raw)
To: Holger Schurig; +Cc: linux-wireless, Holger Schurig, Johannes Berg
On Tue, 10 Nov 2009 16:15:30 +0100, Holger Schurig
<holgerschurig@gmail.com> wrote:
> On Tuesday 10 November 2009 15:50:43 pat-lkml@erley.org wrote:
>> On Tue, 10 Nov 2009 12:51:43 +0100, Holger Schurig
>> <holgerschurig@googlemail.com> wrote:
>>
>> > @@ -3182,20 +3182,11 @@
>> > int err;
>> >
>> > if (!ifidx) {
>> > - err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
>> > - nl80211_fam.attrbuf, nl80211_fam.maxattr,
>> > - nl80211_policy);
>> > - if (err)
>> > - return err;
>> > -
>> > - if (!nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX])
>> > - return -EINVAL;
>> > -
>> > - ifidx = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX]);
>> > - if (!ifidx)
>> > - return -EINVAL;
>> > + ifidx = nl80211_get_ifidx(cb);
>>
>> do you need an:
>>
>> if(ifidx < 0)
>> return ifidx;
>>
>> here, as you assign it to cb->args[0], which differs from the original
>> behavior.
>
>
> Do you mean
>
> <SNIP>
>
> instead?
>
>
>
> I'm not familiar with this function, but maybe we
> can get away like this:
> <SNIP>
I'm not familiar with the code either. I was just glancing through the
patch
and noticed a change that changed the behavior of the code that was there.
before, cb->args[0] was left un-altered when ifidx = nla_get_u32(...)
failed,
but now it's assigned to unconditionally, then, if ifidx is unasigned, an
error
is returned. I've not looked at the context or callers, but the commit
message
implied to me that there should be no functional changes, which is not the
case.
pat
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-11-10 15:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-10 11:51 [PATCH] cfg80211: introduce nl80211_get_ifidx() Holger Schurig
2009-11-10 14:50 ` pat-lkml
2009-11-10 15:15 ` Holger Schurig
2009-11-10 15:41 ` pat-lkml
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).