Hi Jim,
I have your comments incorporated. For the ones that were not Accepted
or needed explanation I've inlined below.
James Carlson wrote:
If you have access to the SWAN, webrevs are located at:
http://zhadum.east.sun.com/export/ws/ss150715/clearview-libdlpi-port-onnv/webrev/
Not yet reviewed (I'll have to get to them tomorrow):
lib/libuuid/common/etheraddr.c
lib/libuuid/common/etheraddr.h
cmd/cmd-inet/sbin/dhcpagent/dlpi_io.c
94: shouldn't there be a free(msgbuf); return (-1); near here?
The reason for continuing and not returning(-1) is because we are only
informing that there was more data then what was asked for. 'msglen'
was the data size requested but but there was more date on received on
the stream.
cmd/cmd-inet/sbin/dhcpagent/interface.c
122,130,139,357,363: this should be MSG_ERROR, not MSG_ERR.
(MSG_ERR assumes that errno was set, and it appends ": %m" to the
output.) (The one at 370 is correct.)
ACCEPTED
ah..yes, errno can be set by any other process so it would append the
":%%m" to the output.
cmd/cmd-inet/sbin/dhcpagent/packet.c
1331: why is 'arg' a void * rather than just a dhcp_pif_t *? It
looks like all of the callers know that it's a PIF.
Accepted. none of the callers of recv_pkt() use 'arg' except
dhcp_collect_dlpi().
usr/src/cmd/cmd-inet/usr.sbin/in.rarpd.c
132,135: why were these renamed? The functions in question are the
ones that handle RARP and ARP "request" messages, so the original
name seemed reasonable to me.
I was thinking along the line - these function provide RARP and ARP
replies. But on second thought that seems more confusing.
Preserving original name.
407,408: initializers seem to be unused.
dlpi_get_physaddr() needs to be passed a 'saddrlenp' >= DLPI_PHYSADDR_MAX.
'str' is initialized so that it gets malloc'd in _link_ntoa().
511: why have an extra 'saddr' buffer? Why not just pass in 'shost'
here and avoid the extra memcpy at line 540?
This can be done and I've changed it.
655: why was syslog() changed to error()? The former just logs an
error; the latter causes the daemon to abend. (Perhaps not a big
problem, as the rest of the daemon is littered with exit points.)
I was thinking since dlpi_send() wasn't able to send the data, it should
bail out.
But since a RARP_REQUEST is requested again, using syslog and not
exiting out of the daemon would be better.
Change would need the message printed in case 'dflag' is set to change
as well.
854: cstyle: compare pointer against NULL.
1000: as long as you're cleaning things up, please use ANSI C.
1006,1019: could reasonably be const.
Accepted.
-Thanks,
Sagun
--
Sagun Shakya
781.442.7344/ X27344
[EMAIL PROTECTED]
_______________________________________________
networking-discuss mailing list
[email protected]
