On Thu, 2012-03-15 at 20:53 +0200, Eliad Peller wrote:
> On Thu, Mar 15, 2012 at 8:39 PM, Johannes Berg
> <johannes@xxxxxxxxxxxxxxxx> wrote:
> > On Thu, 2012-03-15 at 11:37 -0700, Ben Greear wrote:
> >> Looks like this is the compiler's fault to me..but might be
> >> worth explicitly initializing the sta variable just to make
> >> it quiet.
> >
> > I disagree, doing that will only hide legitimate warnings if somebody
> > changes the code. Initializing for compiler happiness is almost always
> > the worst thing you can do.
> >
> what about
>
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 6cb5361..96fc201 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -3094,7 +3094,6 @@ static int ieee80211_prep_connection(struct
> ieee80211_sub_if_data *sdata,
> struct ieee80211_local *local = sdata->local;
> struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
> struct ieee80211_bss *bss = (void *)cbss->priv;
> - struct sta_info *sta;
> bool have_sta = false;
> int err;
>
> @@ -3107,12 +3106,6 @@ static int ieee80211_prep_connection(struct
> ieee80211_sub_if_data *sdata,
> rcu_read_unlock();
> }
>
> - if (!have_sta) {
> - sta = sta_info_alloc(sdata, cbss->bssid, GFP_KERNEL);
> - if (!sta)
> - return -ENOMEM;
> - }
> -
> mutex_lock(&local->mtx);
> ieee80211_recalc_idle(sdata->local);
> mutex_unlock(&local->mtx);
> @@ -3123,10 +3116,15 @@ static int ieee80211_prep_connection(struct
> ieee80211_sub_if_data *sdata,
>
> if (!have_sta) {
> struct ieee80211_supported_band *sband;
> + struct sta_info *sta;
> u32 rates = 0, basic_rates = 0;
> bool have_higher_than_11mbit;
> int min_rate = INT_MAX, min_rate_index = -1;
>
> + sta = sta_info_alloc(sdata, cbss->bssid, GFP_KERNEL);
> + if (!sta)
> + return -ENOMEM;
> +
> sband = sdata->local->hw.wiphy->bands[cbss->channel->band];
>
> ieee80211_get_rates(sband, bss->supp_rates,
>
>
> it also makes the code more clear, imho.
Yeah, I thought about that for a second, but I'm working on changes in
this area that would essentially require me to revert this again ...
I really want to do the allocation first so the failure path for that is
easy. On the other hand, I'm adding channel type switching (see my RFC
patchset) in the middle of this, and unwinding that would be a larger
change that even touches the device.
What may be possible instead is to replace the second have_sta in the
second if with "if (sta)", which may make Ben's compiler a bit happier,
but I have no way of even testing that since my compiler doesn't
complain to start with.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
