Locking AP table incorrect?

[Pavel Roskin]

Hello!

I'm looking into the sources of CVS HostAP and I don't quite understand
why the AP table is locked when it's accessed for reading, but not for
writing.

Let's for example see prism2_hostapd_add_sta() in hostap_ap.c:

        spin_lock_bh(&ap->sta_table_lock);
        sta = ap_get_sta(ap, param->sta_addr);
        if (sta)
                atomic_inc(&sta->users);
        spin_unlock_bh(&ap->sta_table_lock);

        if (sta == NULL) {
                sta = ap_add_sta(ap, param->sta_addr);
                if (sta == NULL)
                        return -1;
        }

I see that ap_add_sta() locks the table, but the fact is, the lock is
being released for some time.  What happens is:

lock
check if station is already in the list
unlock
# bad things happen if somebody inserts station now
if station is not in the list:
    lock
    add station to the list
    unlock
end if

I think it would be better to move the check that the station is already
in the list inside ap_add_sta() and do it without releasing the lock.  If
necessary, an additional argument would specify if ap_add_sta() should
return an error or update the existing entry.

-- 
Regards,
Pavel Roskin