On Wed, 2014-05-07 at 09:04 +0200, Michal Kazior wrote:
> > However, there's one more thing - one caller of ieee80211_csa_finalize()
> > doesn't check the return value now, I think it probably should. Mind
> > taking a look at that? It should *probably* be the same as the other
> > caller (and not propagate the error), I'd say, so maybe that should be
> > rolled into the function?
>
> Right - the return value of ieee80211_csa_finalize() in
> __ieee80211_channel_switch() should be checked against. I think it
> should be like:
>
> err = ieee80211_csa_finalize(sdata);
> if (err) {
> sdata_info(sdata, "failed to finalize immediate CSA, disconnecting\n");
> cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev, GFP_KERNEL);
>
> /* don't propagate the error here - stop the iface instead since
> there's no way to revert CS now */
> err = 0;
> }
>
> Should I re-post the patch(set?), post a follow up, or are you going
> to update the patch yourself?
Can you post a separate patch on top? I already have it in my tree, but
I can squash it I guess if I really want to.
> I'm not sure what you mean by rolling "that" into the function. You
> want to put the stop_iface() in csa_finalize()? I think it's going to
> look a bit ugly that way.
Well, the only other caller of it right now does the same thing:
err = ieee80211_csa_finalize(sdata);
if (err) {
sdata_info(sdata, "failed to finalize CSA, disconnecting
\n");
cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev,
GFP_KERNEL);
goto unlock;
}
so it seems it would make sense to have that as common code, maybe in
the ieee80211_csa_finalize() function or maybe by changing
ieee80211_csa_finalize() to be __ieee80211_csa_finalize() and making a
new ieee80211_csa_finalize() that does this step?
In fact, then we could get rid of the int return value again, so maybe
better post a new version of this particular patch after all.
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
