Sebastien Roy wrote:
On Wed, 2006-06-07 at 00:02 -0700, Nicolas Droux wrote:
Seb,
This looks really good, I have only a few minor comments.
Thank you very much for the review:
I'm happy with your reponses.
Thanks,
Nicolas.
* usr/src/lib/libdladm/common/libdladm.c
ND-001 215 You need to be able to identify the aggregation from which
you want to retrieve the defined VLANs. With these changes
it seems that it will always look for "aggr0".
ACCEPT
This was caught during testing, and has been fixed.
* usr/src/lib/liblaadm/common/liblaadm.c
ND-002 60 Should update the notes on file syntax if it has changed.
ACCEPT
ND-003 413 Isn't this going to cause a failure if the slash separator
is not present?
REJECT
No, because in that case the first token is the entire string, which is
what is returned by strtok_r() (this differs from the strtok()
behavior). I re-tested this to be sure, and it (liblaadm) behaves as
expected when reading both formats (bge0 or bge0/0, for example) from
the /etc/aggregation.conf file.
* usr/src/pkgdefs/SUNWckr/prototype_com
ND-004 90 mismerge?
ACCEPT
Yes, this has been fixed.
* usr/src/pkgdefs/SUNWckr/prototype_i386
ND-005 89 mismerge?
197
264
371
ACCEPT
Yes, this has also been fixed.
* usr/src/pkgdefs/SUNWckr/prototype_sparc
ND-006 96 mismerge?
205
ACCEPT
Yes, this has been fixed.
* usr/src/uts/common/io/aggr/aggr_port.c
ND-007 172-174 Use {} for multi-line for() block
ACCEPT
* usr/src/uts/common/io/mac/mac.c
ND-008 180 Looks like a gratuitous change
ACCEPT
I'm not sure why this was changed, it will be reverted.
ND-009 1043 Typo: s/zefo/zero
ACCEPT
Even though I think there should be a word "zefo", I'll make it
"zero". ;-)
* usr/src/uts/common/sys/aggr_impl.h
ND-010 134 Could use another bit of the bitmask above
ACCEPT
Thank you!
-Seb
_______________________________________________
networking-discuss mailing list
[email protected]
--
Nicolas Droux, Solaris Kernel Networking
Sun Microsystems, Inc. http://blogs.sun.com/droux
_______________________________________________
networking-discuss mailing list
[email protected]
