On 19 March 2014 09:47, Eliad Peller <eliad@xxxxxxxxxx> wrote:
> On Wed, Mar 19, 2014 at 10:34 AM, Michal Kazior <michal.kazior@xxxxxxxxx> wrote:
>> On 18 March 2014 17:42, Eliad Peller <eliad@xxxxxxxxxx> wrote:
>>> On Tue, Mar 18, 2014 at 3:53 PM, Michal Kazior <michal.kazior@xxxxxxxxx> wrote:
>>>> Relax ieee80211_find_chanctx(). If chanctx
>>>> reservation chandef is compatible with
>>>> current-future assigned interfaces chandef then
>>>> allow it to be used by new interfaces.
>>>>
>>>> Signed-off-by: Michal Kazior <michal.kazior@xxxxxxxxx>
>>>> ---
>>> [...]
>>>
>>>> +static const struct cfg80211_chan_def *
>>>> +ieee80211_chanctx_reserved_chandef(struct ieee80211_local *local,
>>>> + struct ieee80211_chanctx *ctx,
>>>> + const struct cfg80211_chan_def *compat)
>>>> +{
>>>> + struct ieee80211_sub_if_data *sdata;
>>>> +
>>>> + lockdep_assert_held(&local->chanctx_mtx);
>>>> +
>>>> + list_for_each_entry(sdata, &ctx->reserved_vifs,
>>>> + reserved_chanctx_list) {
>>>> + if (!compat)
>>>> + compat = &sdata->reserved_chandef;
>>> this check is redundant.
>>
>> It isn't. The function can be called with compat == NULL. You don't
>> want to call cfg80211_chandef_compatible() with a NULL chandef.
>>
> at least this patch has an explicit check for compat not being NULL
> (before calling the function).
> since this function checks chandef compatibility with current
> reserved chandefs, i don't see why anyone would call it with NULL?
Fair argument if you treat the patch separately from the whole patchset.
This is later called with NULL (patch: implement multi-vif in-place
reservations).
I could make a if (WARN_ON(!compat)) return NULL; guard condition at
the function start but I prefer the current way as it has wider
appliance for code reuse.
Michał
--
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
