unreachable code in airo_set_auth()

unreachable code in airo_set_auth()

[Dan Carpenter]

Smatch (http://repo.or.cz/w/smatch.git) complains about unreachable code 
in airo_set_auth() from drivers/net/wireless/airo.c

We don't actually commit the changes, but instead break on line 6577.  I 
didn't know what was intended there.  Or who the maintainer was.

regards,
dan carpenter

  6567          case IW_AUTH_80211_AUTH_ALG: {
  6568                          /* FIXME: What about AUTH_OPEN?  This API seems to
  6569                           * disallow setting our auth to AUTH_OPEN.
  6570                           */
  6571                          if (param->value & IW_AUTH_ALG_SHARED_KEY) {
  6572                                  local->config.authType = AUTH_SHAREDKEY;
  6573                          } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) {
  6574                                  local->config.authType = AUTH_ENCRYPT;
  6575                          } else
  6576                                  return -EINVAL;
  6577                          break;
  6578  
  6579                          /* Commit the changes to flags if needed */
  6580                          if (local->config.authType != currentAuthType)
  6581                                  set_bit (FLAG_COMMIT, &local->flags);
  6582                  }

Re: unreachable code in airo_set_auth()

[John W. Linville]

On Wed, Apr 08, 2009 at 02:02:41PM +0300, Dan Carpenter wrote:
> Smatch (http://repo.or.cz/w/smatch.git) complains about unreachable code 
> in airo_set_auth() from drivers/net/wireless/airo.c
> 
> We don't actually commit the changes, but instead break on line 6577.  I 
> didn't know what was intended there.  Or who the maintainer was.
> 
> regards,
> dan carpenter
> 
>   6567          case IW_AUTH_80211_AUTH_ALG: {
>   6568                          /* FIXME: What about AUTH_OPEN?  This API seems to
>   6569                           * disallow setting our auth to AUTH_OPEN.
>   6570                           */
>   6571                          if (param->value & IW_AUTH_ALG_SHARED_KEY) {
>   6572                                  local->config.authType = AUTH_SHAREDKEY;
>   6573                          } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) {
>   6574                                  local->config.authType = AUTH_ENCRYPT;
>   6575                          } else
>   6576                                  return -EINVAL;
>   6577                          break;
>   6578  
>   6579                          /* Commit the changes to flags if needed */
>   6580                          if (local->config.authType != currentAuthType)
>   6581                                  set_bit (FLAG_COMMIT, &local->flags);
>   6582                  }

Not sure what tree you're based upon, but the same code is farther
down now.  Anyway:

4be757dd	(Dan Williams	2006-01-30 11:58:00 -0500	6716)		break;
4be757dd	(Dan Williams	2006-01-30 11:58:00 -0500	6717)
4be757dd	(Dan Williams	2006-01-30 11:58:00 -0500	6718)		/* Commit the changes to flags if needed */

Looks like we can blame Dan since he added those lines all in one
patch.  Pay no attention to the person that committed... :-)

Dan, what is the proper fix?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

[PATCH] airo: queue SIOCSIWAUTH-requested auth mode change for next commit

[Dan Williams]

Code was clearly wrong, plus callers expect the mode change to happen as
soon as possible, not dropped on the floor until the next time some
other config value changes and a commit happens.

Signed-off-by: Dan Williams <dcbw@redhat.com>

diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index fc4322c..0bf2779 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -6574,11 +6574,11 @@ static int airo_set_auth(struct net_device *dev,
 				local->config.authType = AUTH_ENCRYPT;
 			} else
 				return -EINVAL;
-			break;
 
 			/* Commit the changes to flags if needed */
 			if (local->config.authType != currentAuthType)
 				set_bit (FLAG_COMMIT, &local->flags);
+			break;
 		}
 
 	case IW_AUTH_WPA_ENABLED: