On Tue, 10 Nov 2009 16:15:30 +0100, Holger Schurig
<holgerschurig@xxxxxxxxx> wrote:
> On Tuesday 10 November 2009 15:50:43 pat-lkml@xxxxxxxxx wrote:
>> On Tue, 10 Nov 2009 12:51:43 +0100, Holger Schurig
>> <holgerschurig@xxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
