On Fri, Apr 17, 2009 at 09:29:33PM +0200, Johannes Berg wrote:
> > + if (changed & WIPHY_PARAM_RTS_THRESHOLD) {
> > + if (local->ops->set_rts_threshold)
> > + local->ops->set_rts_threshold(local_to_hw(local),
> > + wiphy->rts_threshold);
> > + }
>
> That could return an error. (so do the changes the other way around)
I noticed, but ended up following the existing behavior in wext.c. While
that may not be the ideal source for good behavior, at least this is
consistent. Should both of them be changed? What exactly should happen
on error?
> > + if (changed & WIPHY_PARAM_RETRY_SHORT)
> > + rdev->wiphy.retry_short = retry_short;
> > + if (changed & WIPHY_PARAM_RETRY_LONG)
> > + rdev->wiphy.retry_long = retry_long;
> > + if (changed & WIPHY_PARAM_FRAG_THRESHOLD)
> > + rdev->wiphy.frag_threshold = frag_threshold;
> > + if (changed & WIPHY_PARAM_RTS_THRESHOLD)
> > + rdev->wiphy.rts_threshold = rts_threshold;
> > +
> > + result = rdev->ops->set_wiphy_params(&rdev->wiphy, changed);
> > + if (result)
> > + goto bad_res;
> > + }
>
> If that returns an error we need to roll back the values?
I thought about this, but ended up not doing that because the existing
(wext) code seemed to behave in the same way. It is unclear whether the
error there would indicate that some of the parameters were taken into
use, but not all. Unless we provide mechanism for returning that
information (e.g., separate calls for each parameter), I'm not sure what
exactly should be done here. Just leaving the parameters (which were
validated before) in struct wiphy seemed like the safest (and well,
certainly easiest ;-) alternative.
--
Jouni Malinen PGP id EFC895FA
--
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
