diff mbox series

EAP: Remove unreachable code in EAP auth SM

Message ID 20190403121615.4094-1-andrei.otcheretianski@intel.com
State Changes Requested
Headers show
Series EAP: Remove unreachable code in EAP auth SM | expand

Commit Message

Andrei Otcheretianski April 3, 2019, 12:16 p.m. UTC 
From: Ilan Peer <ilan.peer@intel.com>

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
---
 src/ap/wpa_auth.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

Comments

Jouni Malinen April 6, 2019, 1:52 p.m. UTC | #1 
On Wed, Apr 03, 2019 at 03:16:12PM +0300, Andrei Otcheretianski wrote:
> diff --git a/src/ap/wpa_auth.c b/src/ap/wpa_auth.c
> @@ -3302,16 +3302,9 @@ SM_STATE(WPA_PTK, PTKINITDONE)

> -	if (0 /* IBSS == TRUE */) {
> -		sm->keycount++;
> -		if (sm->keycount == 2) {
> -			wpa_auth_set_eapol(sm->wpa_auth, sm->addr,
> -					   WPA_EAPOL_portValid, 1);
> -		}
> -	} else {
> -		wpa_auth_set_eapol(sm->wpa_auth, sm->addr, WPA_EAPOL_portValid,
> -				   1);
> -	}
> +	wpa_auth_set_eapol(sm->wpa_auth, sm->addr, WPA_EAPOL_portValid,
> +			   1);
> +

Why? That IBSS case is documented there for purpose as a reminder for
eventually completing that behavior as defined in the standard.
Andrei Otcheretianski April 7, 2019, 7:36 a.m. UTC | #2 
> On Wed, Apr 03, 2019 at 03:16:12PM +0300, Andrei Otcheretianski wrote:
> > diff --git a/src/ap/wpa_auth.c b/src/ap/wpa_auth.c @@ -3302,16 +3302,9
> > @@ SM_STATE(WPA_PTK, PTKINITDONE)
> 
> > -	if (0 /* IBSS == TRUE */) {
> > -		sm->keycount++;
> > -		if (sm->keycount == 2) {
> > -			wpa_auth_set_eapol(sm->wpa_auth, sm->addr,
> > -					   WPA_EAPOL_portValid, 1);
> > -		}
> > -	} else {
> > -		wpa_auth_set_eapol(sm->wpa_auth, sm->addr,
> WPA_EAPOL_portValid,
> > -				   1);
> > -	}
> > +	wpa_auth_set_eapol(sm->wpa_auth, sm->addr, WPA_EAPOL_portValid,
> > +			   1);
> > +
> 
> Why? That IBSS case is documented there for purpose as a reminder for
> eventually completing that behavior as defined in the standard.
Hi Jouni,

Thinks for a quick reply to our patches..
Some static code analyzers warn about unreachable code.
This code is untouched for more than 10 years, so I assumed nobody will ever get to it anyway :)
Personally I think it's better to replace it with a "TODO" comment rather than keeping dead code.
But, it's completely matter of taste - you can drop this patch if you want.

Andrei
> 
> --
> Jouni Malinen                                            PGP id EFC895FA
diff mbox series

Patch

diff --git a/src/ap/wpa_auth.c b/src/ap/wpa_auth.c
index 616b20592c..03806eb093 100644
--- a/src/ap/wpa_auth.c
+++ b/src/ap/wpa_auth.c
@@ -3302,16 +3302,9 @@  SM_STATE(WPA_PTK, PTKINITDONE)
 		}
 	}
 
-	if (0 /* IBSS == TRUE */) {
-		sm->keycount++;
-		if (sm->keycount == 2) {
-			wpa_auth_set_eapol(sm->wpa_auth, sm->addr,
-					   WPA_EAPOL_portValid, 1);
-		}
-	} else {
-		wpa_auth_set_eapol(sm->wpa_auth, sm->addr, WPA_EAPOL_portValid,
-				   1);
-	}
+	wpa_auth_set_eapol(sm->wpa_auth, sm->addr, WPA_EAPOL_portValid,
+			   1);
+
 	wpa_auth_set_eapol(sm->wpa_auth, sm->addr, WPA_EAPOL_keyAvailable, 0);
 	wpa_auth_set_eapol(sm->wpa_auth, sm->addr, WPA_EAPOL_keyDone, 1);
 	if (sm->wpa == WPA_VERSION_WPA)