Re: [PATCH] net: mac80211: tx.c: be sure of 'sdata->vif.type' mustbe NL80211_IFTYPE_AP when be in NL80211_IFTYPE_AP case

[Johannes Berg]

On Sat, 2013-11-30 at 19:59 +0800, Chen Gang wrote:
> On 11/29/2013 11:38 PM, Johannes Berg wrote:
> > 
> >> +++ b/net/mac80211/tx.c
> >> @@ -1814,8 +1814,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> >>  			break;
> >>  		/* fall through */
> >>  	case NL80211_IFTYPE_AP:
> >> -		if (sdata->vif.type == NL80211_IFTYPE_AP)
> >> -			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> >> +		if (sdata->vif.type != NL80211_IFTYPE_AP)
> >> +			goto fail_rcu;
> >> +		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> > 
> > This change is completely wrong.
> > 
> 
> Oh, it is.
> 
> Hmm... for me, this work flow still can be implemented with a little
> clearer way (at least it will avoid related warning):
> 
> -------------------------diff begin------------------------------
> 
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index c558b24..7076128 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1810,14 +1810,14 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
>  		if (!chanctx_conf)
>  			goto fail_rcu;
>  		band = chanctx_conf->def.chan->band;
> -		if (sta)
> -			break;
> -		/* fall through */
> +		if (!sta)
> +			goto try_next;
> +		break;
>  	case NL80211_IFTYPE_AP:
> -		if (sdata->vif.type == NL80211_IFTYPE_AP)
> -			chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> +		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>  		if (!chanctx_conf)
>  			goto fail_rcu;
> +try_next:

I don't think that's better than the (fairly obvious) fall-through, and
has a pretty odd goto. Also, depending on the compiler, it still knows
the previous case label and doesn't warn.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/