On Tue, Apr 17, 2007 at 12:18:14PM +0800, Quaker Fang wrote:
> >>| DISCUSS
> >>| I am not sure libdladm is MT-safe or not, if so, I'll remove the 
> >>mutex.
> >>
> >libdladm is not mt-safe. but from looking at your daemon event loop 
> >code, I don't see a need for thread safety.
> >I suggest you remove the lock, all the pthread calls, and the #include 
> ><pthread.h>
> 
> event loop is okay, but there are also door_up_calls from kernel, which 
> will cause multiple threads in wpa daemon.
>

if you're truly concerned with thread safety, the locking should be done
at a higher level, e.g. in wpa_event_handler, not when you call into
libdladm. for now, you should remove all the pthread calls from driver_wifi.c.

regarding wpa_event_handler, my opinion is that it's highly unlikely the
same wpa_s would have multiple events concurrently. you may add locking to
wpa_event_handler if you think this isn't true.

> 
> >
> >61-71:
> >better for this to look like:
> >
> >ret = dladm_get_link_attr(ifname, &attr);
> >if (ret != DLADM_STATUS_OK)
> >    return (WPA_STATUS(ret));
> >
> >wl_attrp = &attr.la_wlan_attr;
> >if ((attrp.la_valid & DLADM_WLAN_LINKATTR_WLAN) == 0 ||
> >     (wl_attrp->wa_valid & DLADM_WLAN_ATTR_BSSID) == 0)
> >    return (WPA_STATUS(DLADM_STATUS_FAILED));
> >
> >(void) memcpy(bssid, wl_attrp->wa_bssid.wb_bytes, DLADM_WLAN_BSSID_LEN);
> >return (WPA_STATUS(ret));
> >
> >
> >96-107:
> >this should be rearranged like 61-71 above.
> 
> Consider the pthread_mutex protection and wpa_print debug message, seems 
> it's better
> to keep current codes.
>

you should fix the above if you agree to remove pthread_mutex.


please generate a complete webrev against nevada.

thanks
eric

_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to