On Wed, 09 Jan 2008 13:48:36 -0800
Zhenghui.Xie at Sun.COM wrote:
>
> http://cr.opensolaris.org/~xzh/walkprop/
>
> This webrev includes
> 1. the property-walk-through command
> 2. a fix for the bug Jia pointed out
> 3. a better/user-friendly display format (for instance, it will accept
> and report "true/false" for a boolean value parameter).
>
> Thanks for review!
>
> BTW, in case anybody curious, the up-to-date nwamcfg(1M) man page is on
> the way...
nwamcfg.c:313,335 what is this suppose to be? maybe
sizeof(ncu_prop_table)/sizeof(ncu_prop_table[0])
way nit: personally I like SIZE or NELEMS for number of elements in a
table and LEN for length of buffers because I think they carry more of
the meaning then NUM
nwamcfg.c:826 boolean_t
nwamcfg.c:894-6 nit: Above (896,8,...) you enclose single line blocks.
Here in an else with a single line unbraced block and a braced else
block. I think you should make these consistent. Personal choice is
to always use blocks unless there is some if/else if/.../else block all
with single line bodies but consistency is the more important goal.
nwamcfg.c:910-915 way nit: All of the inner structure initializers have
the extra ',' at the end. That looks a little wierd. The one after the
last array element on 915 makes some sense and is specially allowed in
the ANSI c specification due to automatic code generators FWIR. In our
less mechanical code writing duties it means you can add elements to
the array without changing the last line thus limiting the size of your
diff. Yea, seems like a lot of thought for pretty modest returns.
nwamcfg.c:1377 wonder if you should have a define for whatever an
inband bad nwam_data_type_t is.
nwamcfg.c:1404 const char *input_str?
nwamcfg.c:1537,1573 nit like 894 above
nwamcfg.c:2275,2314 The input is not guaranteed to be newline termianted.
libnwam_ncp.c:1122 you might assert that i doesn't get > prop_num
mph
>
> -Jan
> _______________________________________________
> nwam-discuss mailing list
> nwam-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
>
