[OpenWrt-Devel] [PATCH] iwinfo: do not wait for scan results if scan request failed.

[John Crispin]

On 20/08/2015 10:06, Dmitrijs Ivanovs wrote:
> Hi John!
> 
> 1) The code is reachable because "err" is signed integer. Negative
> value is error indication.

of course ... i read the while as "err will be 0 when it leaves the loop"

> 2) Cleanup is called later in all cases. Actually, calling it here
> leads to an error - a kind of "double free". It was never encountered
> because it was never called in past.
> 

ok, just looked at the patch without the context

> I had to create this patch after transition to the new kernel because
> AP scan patch was accidentally removed. Inability of kernel to do wifi
> scan in AP mode caused this situation where libiwinfo had to wait
> indefinitely for scan results. Ok, AP scan patch is restored now, but
> libiwinfo should be more robust against similar situations in future.
> In fact, I have tested my patch with both AP scan turned on and off.

fair enough, will have a closer look later, thanks




> On Mon, Aug 17, 2015 at 1:18 PM, John Crispin <blogic at openwrt.org> wrote:
>> Hi,
>>
>> I have various issues with this patch
>>
>> On 31/07/2015 10:53, Dmitry Ivanov wrote:
>>> Do not wait for scan results if scan request failed.
>>>
>>> Signed-off-by: Dmitry Ivanov <dima at ubnt.com>
>>> ---
>>>  iwinfo_nl80211.c | 14 ++++++++++----
>>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c
>>> index 900eef2..251ec33 100644
>>> --- a/iwinfo_nl80211.c
>>> +++ b/iwinfo_nl80211.c
>>> @@ -389,11 +389,12 @@ static struct nl80211_msg_conveyor * nl80211_send(
>>>       while (err > 0)
>>>               nl_recvmsgs(nls->nl_sock, cv->cb);
>>>
>>> +     if (err)
>>> +             goto err;
>>> +
>>
>> this code will never be reached due tot he while (err > 0) above
>>
>>
>>>       return &rcv;
>>>
>>>  err:
>>> -     nl_cb_put(cv->cb);
>>> -     nlmsg_free(cv->msg);
>>>
>>
>> removing the freeeing code from a global helper is not a good idea and
>> will cause a leak.
>>
>>
>>         John
>>
>>
>>
>>>       return NULL;
>>>  }
>>> @@ -2016,16 +2017,21 @@ static int nl80211_get_scanlist_cb(struct nl_msg *msg, void *arg)
>>>
>>>  static int nl80211_get_scanlist_nl(const char *ifname, char *buf, int *len)
>>>  {
>>> -     struct nl80211_msg_conveyor *req;
>>> +     struct nl80211_msg_conveyor *req, *scan_res = NULL;
>>>       struct nl80211_scanlist sl = { .e = (struct iwinfo_scanlist_entry *)buf };
>>>
>>>       req = nl80211_msg(ifname, NL80211_CMD_TRIGGER_SCAN, 0);
>>>       if (req)
>>>       {
>>> -             nl80211_send(req, NULL, NULL);
>>> +             scan_res = nl80211_send(req, NULL, NULL);
>>>               nl80211_free(req);
>>>       }
>>>
>>> +     if (!scan_res)
>>> +     {
>>> +             return -1;
>>> +     }
>>> +
>>>       nl80211_wait("nl80211", "scan", NL80211_CMD_NEW_SCAN_RESULTS);
>>>
>>>       req = nl80211_msg(ifname, NL80211_CMD_GET_SCAN, NLM_F_DUMP);
>>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel at lists.openwrt.org
>> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
> 
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel