On Wed, 2016-02-17 at 11:47 +0100, Arend van Spriel wrote:
>
> + * @NL80211_ATTR_BSS_SELECT: nested attribute for driver supporting the
> + * BSS selection feature. When used with %NL80211_CMD_GET_WIPHY it contains
> + * NLA_FLAG type according &enum nl80211_bss_select_attr to indicate what
> + * BSS selection behaviours are supported. When used with %NL80211_CMD_CONNECT
> + * it contains the behaviour and parameters for BSS selection to be done by
> + * driver and/or firmware.
Maybe we should be less specific here regarding the NLA_FLAG and say
that it will contain attributes for each, indicating which behaviours
are supported, and say there's behaviour-dependent data inside each of
those attributes?
It seems entirely plausible that future behaviours would require their
own sub-capabilities; perhaps that's even the case today for having an
adjustment range for example, not that I think it's really necessary
now.
> @@ -3617,6 +3624,7 @@ enum nl80211_band {
> NL80211_BAND_2GHZ,
> NL80211_BAND_5GHZ,
> NL80211_BAND_60GHZ,
> + NUM_NL80211_BAND,
You should use IEEE80211_NUM_BANDS and remove this.
> +static bool is_band_valid(struct wiphy *wiphy, enum nl80211_band b)
> +{
> + return b < NUM_NL80211_BAND && wiphy->bands[b];
> +}
Here.
> +static int parse_bss_select(struct nlattr *nla, struct wiphy *wiphy,
> + struct cfg80211_bss_selection
> *bss_select)
> +{
> + struct nlattr *attr[NL80211_BSS_SELECT_ATTR_MAX + 1];
> + struct nlattr *nest;
> + int err;
> + bool found = false;
> + int i;
> +
> + /* only process one nested attribute */
> + nest = nla_data(nla);
> + if (!nla_ok(nest, nla_len(nest)))
> + return -EBADF;
This isn't quite clear to me. Shouldn't you do this by declaring it
NLA_TESTED in the nl80211_policy?
Actually, not really? you use nla_ok() on the inner (without having
checked at all that it's even long enough btw, since you didn't do the
policy change), but that would already be done by nla_parse() below?
What are you trying to do here?
Regardless of that, EBADF seems like a bad error number.
> + err = nla_parse(attr, NL80211_BSS_SELECT_ATTR_MAX,
> nla_data(nest),
> + nla_len(nest), nl80211_bss_select_policy);
> + if (err)
> + return err;
> +
> + /* only one attribute may be given */
> + for (i = 0; i <= NL80211_BSS_SELECT_ATTR_MAX; i++)
> + if (attr[i]) {
> + if (found)
> + return -EINVAL;
> + found = true;
> + }
I'd kinda prefer braces, but maybe that's just me. Surely not a
blocking issue :)
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
