sagun shakya writes: > If you have access to the SWAN, webrevs are located at: > http://zhadum.east.sun.com/export/ws/ss150715/clearview-libdlpi-port-onnv/webrev/
Reviewed, no comments: cmd/cmd-inet/sbin/dhcpagent/Makefile cmd/cmd-inet/sbin/dhcpagent/packet.h cmd/cmd-inet/sbin/dhcpagent/request.c cmd/cmd-inet/sbin/dhcpagent/select.c cmd/cmd-inet/usr.sbin/Makefile cmd/dladm/dladm.c cmd/zoneadm/Makefile cmd/zoneadm/zoneadm.c lib/Makefile lib/libdhcpagent/common/dhcp_stable.c lib/libdhcputil/common/dhcp_inittab.c lib/libdhcputil/common/dhcp_inittab.h lib/libdhcputil/common/mapfile-vers lib/libdladm/Makefile lib/libdladm/amd64/Makefile lib/libdladm/common/linkprop.c lib/libdladm/sparcv9/Makefile lib/libdlpi/common/libdlpi.c lib/libdlpi/common/libdlpi.h lib/libdlpi/common/mapfile-vers lib/libuuid/Makefile.com pkgdefs/SUNWcslr/prototype_i386 pkgdefs/SUNWcslr/prototype_sparc pkgdefs/etc/exception_list_i386 pkgdefs/etc/exception_list_sparc deleted_files/usr/src/cmd/cmd-inet/sbin/dhcpagent/dlprims.c usr/src/cmd/cmd-inet/sbin/dhcpagent/dlprims.c deleted_files/usr/src/cmd/cmd-inet/sbin/dhcpagent/dlprims.h usr/src/cmd/cmd-inet/sbin/dhcpagent/dlprims.h deleted_files/usr/src/cmd/zoneadm/dlprims.c usr/src/cmd/zoneadm/dlprims.c 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 56: indenting is off. 94: shouldn't there be a free(msgbuf); return (-1); near here? 100: cast not needed cmd/cmd-inet/sbin/dhcpagent/dlpi_io.h 46: comment is no longer true -- it's just the timeout for pushing the filter module. 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.) cmd/cmd-inet/sbin/dhcpagent/interface.h 73,74: as the block comment explaining things is now gone, these comments should probably say something like "our L2 destination." 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. 1360: missing blank line after declaration. 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. 407,408: initializers seem to be unused. 482,492,493,630,741: cast unnecessary. 511: why have an extra 'saddr' buffer? Why not just pass in 'shost' here and avoid the extra memcpy at line 540? 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.) 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. 1017: this would probably be better up at the forward declaration (line 144). -- James Carlson, Solaris Networking <james.d.carlson at sun.com> Sun Microsystems / 1 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677
