... which get's rid of three indentical cut-n-paste sections.
Signed-off-by: Holger Schurig <[email protected]>
---
... 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
On Tue, 10 Nov 2009 16:15:30 +0100, Holger Schurig
<[email protected]> wrote:
> On Tuesday 10 November 2009 15:50:43 [email protected] wrote:
>> On Tue, 10 Nov 2009 12:51:43 +0100, Holger Schurig
>> <[email protected]> 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
On Tuesday 10 November 2009 15:50:43 [email protected] wrote:
> On Tue, 10 Nov 2009 12:51:43 +0100, Holger Schurig
> <[email protected]> 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
On Tue, 10 Nov 2009 12:51:43 +0100, Holger Schurig
<[email protected]> 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)