[PATCH] mac80211: clean up an ieee80211_do_open error path

[PATCH] mac80211: clean up an ieee80211_do_open error path

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Eliad's comment prompted me to look closer at
the error paths in ieee80211_do_open() and I
found one that should use the error labels.

Also add a comment about the clear_bit since
in many error cases the bit hasn't been set.

Cc: Eliad Peller <eliad@wizery.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/iface.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/net/mac80211/iface.c	2012-04-03 14:41:44.000000000 +0200
+++ b/net/mac80211/iface.c	2012-04-03 14:44:24.000000000 +0200
@@ -349,9 +349,8 @@ static int ieee80211_do_open(struct net_
 		memcpy(dev->perm_addr, dev->dev_addr, ETH_ALEN);
 
 		if (!is_valid_ether_addr(dev->dev_addr)) {
-			if (!local->open_count)
-				drv_stop(local);
-			return -EADDRNOTAVAIL;
+			res = -EADDRNOTAVAIL;
+			goto err_stop;
 		}
 	}
 
@@ -485,6 +484,7 @@ static int ieee80211_do_open(struct net_
 	sdata->bss = NULL;
 	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
 		list_del(&sdata->u.vlan.list);
+	/* might already be clear but that doesn't matter */
 	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 	return res;
 }

Re: [PATCH] mac80211: clean up an ieee80211_do_open error path

[Johannes Berg]

On Tue, 2012-04-03 at 14:45 +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Eliad's comment prompted me to look closer at
> the error paths in ieee80211_do_open() and I
> found one that should use the error labels.

Note that in practice it doesn't matter.

> +++ b/net/mac80211/iface.c	2012-04-03 14:44:24.000000000 +0200
> @@ -349,9 +349,8 @@ static int ieee80211_do_open(struct net_
>  		memcpy(dev->perm_addr, dev->dev_addr, ETH_ALEN);
>  
>  		if (!is_valid_ether_addr(dev->dev_addr)) {
> -			if (!local->open_count)
> -				drv_stop(local);
> -			return -EADDRNOTAVAIL;
> +			res = -EADDRNOTAVAIL;
> +			goto err_stop;
>  		}

This calls drv_stop() etc, so the only thing we don't clean up here is

>  	sdata->bss = NULL;
>  	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
>  		list_del(&sdata->u.vlan.list);
> +	/* might already be clear but that doesn't matter */
>  	clear_bit(SDATA_STATE_RUNNING, &sdata->state);

the BSS clearing and the state bit. Neither of those can actually be
relevant though since AP_VLAN interfaces can't be brought up as the
first ones since they're slaved to an AP interface...

Still better to clean up, but no reason to bother stable.

johannes