On Wed, Dec 02, 2009 at 08:55:11AM -0500, Anurag S. Maskey wrote: > Renee Danson Sommerfeld wrote: > >>requesting code review for the following bugs: > >> > >> 9012 It's better if the output of 'nwamcfg list' and 'nwamadm list' > >> is not random sequence. > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=9012 > >> > >> 11548 nwamcfg says lies about nothing to commit > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=11548 > >> > >> 11987 nwamcfg should have a way to display all available properties > >> for an object > >> http://defect.opensolaris.org/bz/show_bug.cgi?id=11987 > >> > >>http://zhadum.east/export/ws/am223141/checkout-area/nwam1-fixes/webrev/ > >nwamcfg.c > >1164: This seems like an odd condition for whether or not we should > > print the message; I thought the condition was going to be > > whether or not the user explicitly performed the commit (i.e. > > she typed the 'commit' command, versus having the commit > > happen implicitly because she typed 'end'). The value of > > interactive_mode would be true in both cases, wouldn't it? > This wasn't happening because commit_func() was called by end_func() > only if there was something to commit. > > However, I did make a change so that do_commit() is the function > that does the libnwam calls to nwam_*_commit(). It is also called > by commit_func(). end_func() calls do_commit() directly, so the > message is not printed. This approach is cleaner than relying on > commit_func() only being called if there is something to commit. > Now, if commit_func() is only called when "commit" is explicitly > typed. All implicit commits go directly to do_commit(). (Also, made > similar change to cancel_func() and do_cancel().) webrev updated.
Sounds good. > >libnwam_object.c > >name_cmp(): If I'm reading this code correctly, it would sort a > > list of two link/interface pairs as: > > > > TYPE PROFILE STATE ncp User > >online ncu:ip e1000g0 online ncu:phys > >e1000g0 online ncu:ip wpi0 offline > > ncu:phys wpi0 offline > > > > I like that the link/interface pairs are grouped; but I > > think the ordering should be swapped (i.e. don't sort the > > ip vs. phys part alphabetically). It seems more logical > > to me for the thing that has to come up first (the link) to be > >listed first. > Nope, it's the other way. The link ncus are listed before the > interface ncus. The reason is that nwam_ncu_type_t has _LINK listed > before _INTERFACE, so ncu:phys appear before ncu:ip. When the NCU > type/class is not specified, the code explicitly lists _LINK before > _INTERFACE. Cool, the sample output you sent looks good. Thanks for clarifying! -renee
