Search Linux Wireless

Re: [PATCH v3]hostap:hostap_ioctl.c Fix variable 'ret' set but not used

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/02/2010 08:31 PM, Julian Calaby wrote:
On Tue, Aug 3, 2010 at 12:49, Justin P. Mattock<justinmattock@xxxxxxxxx>  wrote:
On 08/02/2010 07:31 PM, Julian Calaby wrote:

On Tue, Aug 3, 2010 at 12:23, Justin P. Mattock<justinmattock@xxxxxxxxx>
  wrote:

wait im lost.. looking at your patch looks the same(number wise) except I
see different parts of code in there:

hostap_set_word(dev, HFA384X_RID_CNFROAMINGMODE,
                                HFA384X_ROAMING_FIRMWARE);

-       return 0;
+       return ret;



mine is:

printk(KERN_DEBUG "SCANREQUEST failed\n");
-               ret = -EINVAL;
+               return ret;


The tree Im using is Linus's tree.

I'm using wireless-testing, and the line you're changing is #1692, I'm
changing #1699.

Thanks,



ahh.. so there's two return ret's that need to be added then..
(if so then go ahead and take my patch and add it to yours.. I'm off to
other things)

No.

The function works like this:

1. set up scan_req
2. if local->host_roaming is not set, set host roaming on the hardware
3. set scan_req on the hardware
4. if local->host_roaming is not set, set the hardware to do firmware roaming
5. If set_req could not be set on the hardware, return -EINVAL,
otherwise return 0.

The significant bits are steps 2 and 4. These must *both* be executed,
regardless of whether setting scan_req on the hardware succeeds or
not.

As such, the return value is stored in the ret variable, which is
initialised to 0 initially, then set to -EINVAL if setting scan_req on
the hardware fails. Hence, step 5 can be performed by returning ret.

As such, all of your patches are wrong as they are either returning
the wrong value when setting scan_req on the hardware fails, or
failing to execute step 4. The *only* correct place to return a value
is once the roaming status of the hardware has been set properly -
i.e. at the end of the function.

I hope this has cleared up your confusion.

Thanks,



cleared it up perfectly... didnt think that using ret down more would work..(was stuck a few lines up) but it does.. just tested your version out and no warning messages..

cheers,

Justin P. Mattock
--
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


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux