On 23 June 2014 15:03, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
> So, I was going to apply this, but then I ran sparse & smatch :-)
>
> On Tue, 2014-06-17 at 14:58 +0200, Michal Kazior wrote:
>
>> - curr_ctx = container_of(conf, struct ieee80211_chanctx, conf);
>> + curr_ctx = ieee80211_vif_get_chanctx(sdata);
>> + if (curr_ctx && local->use_chanctx && !local->ops->switch_vif_chanctx)
>> + return -ENOTSUPP;
>
> curr_ctx can be NULL?
>
>> new_ctx = ieee80211_find_reservation_chanctx(local, chandef, mode);
>> if (!new_ctx) {
>> + if (ieee80211_can_create_new_chanctx(local)) {
>> new_ctx = ieee80211_new_chanctx(local, chandef, mode);
>> + if (IS_ERR(new_ctx))
>> + return PTR_ERR(new_ctx);
>> } else {
>> + if ((curr_ctx->replace_state ==
>> + IEEE80211_CHANCTX_WILL_BE_REPLACED) ||
>
> then this crashes?
Right. Thanks to this I've just found another issue with unreserving
(if in-place reservation vifs are unreserved+unassigned then
(re)assign reservations need special treatment).
>> + if (local->use_chanctx) {
>> + vif_chsw = kzalloc(sizeof(*vif_chsw) * n_vifs, GFP_KERNEL);
>> + if (vif_chsw) {
>
> clearly you didn't test this? :)
:(
>> + err = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + i = 0;
>> + list_for_each_entry(ctx, &local->chanctx_list, list) {
>> + if (ctx->replace_state != IEEE80211_CHANCTX_REPLACES_OTHER)
>> + continue;
>> +
>> + if (WARN_ON(!ctx->replace_ctx))
>> + return -EINVAL;
>
> This also leaks "vif_chsw".
This should be a `goto` too.
I'll re-spin once I fix all the problems that I'm aware of now.
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
