On Thu, 2024-03-28 at 12:59 +0530, Karthikeyan Periyasamy wrote:
>
> +static int
> +cfg80211_validate_iface_comb_limits(struct wiphy *wiphy,
> + struct iface_combination_params *params,
> + const struct ieee80211_iface_combination *c,
> + u16 num_interfaces, u32 *all_iftypes)
> +{
> + struct ieee80211_iface_limit *limits;
> + int ret = 0;
That initialization is useless.
> +
> + if (num_interfaces > c->max_interfaces)
> + return -EINVAL;
> +
> + if (params->num_different_channels > c->num_different_channels)
> + return -EINVAL;
> +
> + limits = kmemdup(c->limits, sizeof(limits[0]) * c->n_limits,
> + GFP_KERNEL);
> + if (!limits)
> + return -ENOMEM;
> +
> + ret = cfg80211_validate_iface_limits(wiphy,
> + params->iftype_num,
> + limits,
> + c->n_limits,
> + all_iftypes);
> +
> + kfree(limits);
> +
> + return ret;
> +}
> +
> +static u16 cfg80211_get_iftype_info(struct wiphy *wiphy,
> + const int iftype_num[NUM_NL80211_IFTYPES],
> + u32 *used_iftypes)
> +{
> + enum nl80211_iftype iftype;
> + u16 num_interfaces = 0;
> +
This should probably set *used_iftypes = 0.
> + for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
> + num_interfaces += iftype_num[iftype];
> + if (iftype_num[iftype] > 0 &&
> + !cfg80211_iftype_allowed(wiphy, iftype, 0, 1))
> + *used_iftypes |= BIT(iftype);
and that could really use a rewrite like
if (!iftype_num[iftype])
continue;
num_interfaces += ...
if (!allowed...)
*used_iftypes |= ...;
I'd say.
> for (i = 0; i < wiphy->n_iface_combinations; i++) {
> const struct ieee80211_iface_combination *c;
> - struct ieee80211_iface_limit *limits;
> u32 all_iftypes = 0;
>
> c = &wiphy->iface_combinations[i];
>
> - if (num_interfaces > c->max_interfaces)
> - continue;
> - if (params->num_different_channels > c->num_different_channels)
> + ret = cfg80211_validate_iface_comb_limits(wiphy, params,
> + c, num_interfaces,
> + &all_iftypes);
> + if (ret == -ENOMEM) {
> + break;
> + } else if (ret) {
> + ret = 0;
> continue;
Seems that 'break' is equivalent to just 'return ret'? And that setting
ret = 0 seems ... strange.
> - return 0;
> + return ret;
> }
And then you don't need that either which is much nicer anyway...
johannes
