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 <[email protected]> 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 <[email protected]> >>> --- >>> 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 >> [email protected] >> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel > _______________________________________________ > openwrt-devel mailing list > [email protected] > https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel > _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
