Here's the remainder of my comments, on dladm.c
-renee
-------------------------------------------------------------------------------
usr/src/cmd/dladm/dladm.c
-------------------------------------------------------------------------------
RD-06 202-3, 208 Cos Total nit: why is the whitespace change needed?
RD-07 old 1866 Func Why was this line (call to kstat_close() in
get_single_mac_stat()) removed? Seems like it
shouldn't have been.
RD-08 2057 Func lasts should be initialized to NULL.
RD-09 2095-2100 Perf If you find a matching name, but the cmdtype is
wrong, no need to continue looping. I would
separate out the cmdtype check, so if the name
matches but it doesn't you can bail.
RD-10 do_connect_wifi Func I think there's a potential error case missed
here. If the user specifies a security mode
(other than wep) and also specifies a wep key,
the 'k' option handling will override the 's'
option handling, and things will proceed as if
the -s option hadn't been included. That case
should result in an error, I think.
RD-11 2759 Func duplicate of line 2752
RD-12 convert_secobj() Com There are several seemingly magic numbers in
this function (the checks on lines 3175 and
3184, for example). Some comments would be
useful; using #defines for those numbers could
help, as well.
RD-13 audit_*() CodeStd audit_create() and audit_delete() are almost
identical; could they be collapsed to one, with
a differentiating parameter?
RD-14 3532, 3620 CodeStd Probably want to or this flag in, rather than
explicitly setting; it's okay for now, as we
only deal with one flag, but if there are more
in the future, this could be a problem.
RD-15 3686 Design Do we really intend to leave in the ability to
dump out the secure values? This doesn't seem
good to me.
Comment type key:
Func comment on functionality
Perf comment on performance
CodeStd comment on coding standards
Design comment on design
Edit editorial comment
Cos cosmetic comment
Com comment on missing comments
_______________________________________________
networking-discuss mailing list
[email protected]
