the following comments refer to the webrev at:
http://cr.grommit.com/~zf162725/cr_0308/
usr/src/cmd/dladm/dladm.c
2070:
you are using this for parsing wpa keys so it shouldn't be
called parse_wep_keys() anymore. it's better to call it
parse_wlan_keys(). also, wladm_wep_key_t should be renamed to
wladm_key_t.
238:
change this to:
" [-s wep|wpa]\n"
2217-2227:
these lines can be replaced with:
die("key required for security mode '%s'",
wladm_secmode2str(&attr.secmode, buf));
3073:
change string to:
"valid values are: wep, wpa"
3186:
change "wep" to "unknown"
usr/src/lib/libwladm/common/libwladm.h:
159-163:
please check if these #defines are valid for wpa or not.
if so, remove the 'WEP' from the names.
if not, you'll likely need a new wladm_wpa_key_t.
usr/src/lib/libwladm/common/libwladm.c:
45-47:
which functions need these #includes?
2355:
you should move this function to 2239 so you won't have to
declare it at 72.
522:
shouldn't this be in wpa.h instead?
(btw, wpa.h is not in the webrev)
564-570,596-597:
can these be moved outside of do_connect(), maybe to 653?
if you do this, you can remove the link argument to do_connect().
2027:
delete this comment.
2289,2294-2295:
username seems to be always 'root'.
you should use getpid() and getpwuid() to get the current username.
the following comments refer to the webrev at:
http://cr.grommit.com/~zf162725/cr_0322/
usr/src/lib/libwladm/common/libwladm.h:
195-203:
which one of these interfaces are wpa specific?
the wpa specific interfaces should be renamed like so:
wladm_wpa_get_sr(), wladm_wpa_set_ie(),...
wladm_cmd_scan(), wladm_get_bssid(), wladm_get_essid()
are not really necessary because they seem to be implementable
by extending the existing interfaces.
for wladm_cmd_scan(), you can just change wladm_scan() to
check if func is NULL and only do the scan if it is.
for get_bssid,essid, you should just use wladm_get_link_attr()
and extend the wladm_link_attr_t with the fields la_bssid and
la_essid. these represent the local bssid/essid. corresponding
WLADM_LINK_ATTR_ESSID/BSSID flags need to be added as well.
also, I don't like the exposure of the wl_* data structures to
clients of libwladm. for each one of these structures, you
should create a similar one called wladm_xxx; and in your new
wladm routines, you translate these types to your wl_xxx types.
the reason for doing this is to keep libwladm clients insulated
from ioctl interface changes.
once you've done the above, the <net/wpa.h> include can be removed.
uts/common/net/wpa.h:
this file is not in your webrevs yet.
I noticed there are lots of #defines here prefixed with IEEE80211.
are these supposed to be part of net80211.h?
117,144,174:
these structure names seem to be inconsistent with the typedef'ed name.
e.g. struct ieee80211req_key should be struct wl_key instead.
This message posted from opensolaris.org
_______________________________________________
networking-discuss mailing list
[email protected]
