> webrev: http://cr.grommit.com/~zf162725/cr_0308/
> workspace & cscope: /net/greatwall.prc/workspace/wifi-wpa-cr/usr/src
>
Hi Quaker,
I only looked at the 5 and 6 parts of the changes:
usr/src/cmd/dladm/dladm.c
usr/src/lib/libdladm/common/libdladm.h
usr/src/lib/libdladm/common/secobj.c
usr/src/uts/common/io/dld/dld_drv.c
usr/src/uts/common/sys/dld.h
usr/src/lib/libwladm/Makefile.com
No Comments
usr/src/lib/libwladm/common/libwladm.c (not including the smf changes)
1. I don't like the fact that do_connect() and do_disconnect() takes two
arguments fd and link which refers to basically the same object. I
understand that the fundamental problem behind this is libwladm trying to
reuse fd for every operation, but which I think it is unnecessary.
There are some other problems and inconsistencies - we don't need to reuse
gbuf in every functioins libwladm calls. Another example is some of do_xxx()
function returns wladm_status_t and some of them returns int.
So ideally, for example, I prefer the do_get_capability() function to be in
the form of:
wladm_status_t do_get_capability(const char *link, wl_capability_t *cap);
I understand some of these problems already existed before your changes, but
as you are the same project team working on the libwladm code before, I hope
you can make the necessary changes so that this project and the future ones
won't add some strange code to work around these problems.
2. do_connect()
Why not call wpa_instance_delete() when connecting attempt fails or timeout
on L602 and L608?
3. wladm_ioctl_set() and wladm_ioctl_get()
I don't think it is right to introduce these two generic libwladm APIs (Do
they PSARCed?) I think it makes more sense to have library functions like:
wladm_set_wpa(), wladm_del_key() etc.
usr/src/lib/libwladm/common/libwladm.h
usr/src/lib/libwladm/common/mapfile-vers
The same as 3.
Thanks
- Cathy
