[PATCH] Add ssids_equal() to compare SSIDs

Jouni Malinen j at w1.fi
Sun Jan 4 05:49:28 EST 2015


On Mon, Dec 15, 2014 at 01:23:31PM +0100, Stefan Tomanek wrote:
> This change moves the common task of comparing both length values and the
> corresponding byte arrays into a generic function call taking both ssid
> pointers and lengths as arguments.

Why? And how much did you test the changes?

>  15 files changed, 47 insertions(+), 75 deletions(-)

This would kind of look nice, but if you'd clean up the coding style to
match the style used in hostap.git, (<= 80 char lines), this is actually
increasing the number of lines.. Even worse, this breaks most of
wpa_supplicant functionality due to incorrect conversion. I don't think
I see enough benefit in this to justify the risk of regressions if this
is based on manual edits rather than something automated like spatch
with a semantic patch.

At least this one is incorrect.. I stopped reviewing here taken into
account I'm not sure I'd apply this anyway in the end:

> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> @@ -817,9 +817,7 @@ static struct wpa_ssid * wpa_scan_res_match(struct wpa_supplicant *wpa_s,
>  		    os_memcmp(bss->bssid, ssid->bssid, ETH_ALEN) == 0)
>  			check_ssid = 0;
>  
> -		if (check_ssid &&
> -		    (bss->ssid_len != ssid->ssid_len ||
> -		     os_memcmp(bss->ssid, ssid->ssid, bss->ssid_len) != 0)) {
> +		if (check_ssid && ssids_equal(ssid->ssid, ssid->ssid_len, bss->ssid, bss->ssid_len)) {
>  			wpa_dbg(wpa_s, MSG_DEBUG, "   skip - SSID mismatch");
>  			continue;
>  		}

Had you ran even a single test with this, you'd seen no connection
working properly..
 
-- 
Jouni Malinen                                            PGP id EFC895FA


More information about the HostAP mailing list