Girish Moodalbail wrote:
On 10/12/09 22:16, Garrett D'Amore wrote:
All,
I've converted hme to the common MII framework (and hence to
Brussels). This fixes at least one bug (it didn't work at forced 10
Mbps!), and cleans up a *a lot* of code. hme shrinks by about 2200
lines.
The webrev is here:
http://cr.opensolaris.org/~gdamore/hme-mii
Hello Garrett,
I have looked at only Brussels interfaces and here are my comments:
hme.c:
L123-L128: Since all the 4 private properties are per-interface, the
comments above those lines are incorrect. They talk about making these
parameters per-interface (which is already per-interface) and using
ndd command.
Good catch. I"m just remove the comment.
L130-133: Do we really need these to be global variables? They are
used only for initiation and so can be represented as macros right.
They could be macros, but making them globals also allows them to be
adjusted via /etc/system. I'm not going to change them for now, because
there simply isn't any compelling need to do so.
L1917: rv should be reset to 0. otherwise when everything is fine we
will return (ENOTSUP) or else @L1950 return (ENOTSUP) and L1956 return
(0)
Accept. I guess I didn't try changing these parameters. Need to cover
that with more testing.
L1932: There is a missing 'else' clause.
Accept, good catch.
L72, L90, L127, L269: NIT: no need for ','
Yeah, but C is ok with it either way. I like to leave the last , in
place, so if another member gets added to the end, you don't have to
remember to add one without creating a syntax error. As this is mostly
a stylistic preference, I'm going to leave it alone for now.
Thanks for reviewing this stuff.
- Garrett
~Girish
_______________________________________________
networking-discuss mailing list
[email protected]
