Thank you Meem.
All of the comments provided below have been addressed and fixed.
A webrev with the changes incorporated can be found at:
http://cr.grommit.com/~sshakya/libdlpi_webrev_followup/
-Sagun
Peter Memishian wrote:
> If you're off the SWAN webrevs are located at:
> webrev against the ONNV gate:
> http://cr.grommit.com/~sshakya/libdlpi-webrev-baseon/
Some more comments. Sorry for sending these in bits and pieces, but I've
been working through it in my spare cycles.
libdlpi.c:
* 457: For consistency, use `addrlen', not `addr_length'.
* 607: dl_ prefix for dl_sap seems needless; `sap' seems just
as clear for a stack-local variable.
* 608: No need for dl_sapaddrlen. Remove it and just
change 647 to:
datareqp->dl_dest_addr_length = daddrlen + dip->dli_saplen;
* 640-644: This comment doesn't make sense to me -- why would the
full address be incorrect? Nonetheless, we still should enforce
that must be daddrlen > DLPI_PHYSADDR_MAX, but it should be done
at the top of the function (~line 613) and it should only be
done if daddrp != NULL.
* 660: Remove useless cast of `&udatareqp[1]'.
* 682-683: Not sure why this code is before the raw mode
handling; seems more natural on line 697.
* 717-718: This check belongs at the top, not buried down here
after we've already done work.
* 792: Remove blank line.
* 814-815: The defined interface is that the integer returned is
the style, not the style or an error number. As such, this
error handling code should be removed.
* 1309, 1349: s/code/codes/
* 1311-1345: Why do some of these error messages begin with a
capital letter?
* 1330: This entry is incorrect; DL_BOUND was already listed on
line 1328. I'd presume none of the following entries in the
table can be properly accessed.
* 1331: s/alternat/alternate/
* 1342: s/ussuported/unsupported/
* 1349: s/specific//
* 1385-1412: Consider adding a tab after the first column so that
the table columns stay aligned.
* 1410-1411: The word "Link" in here seems redundant.
--
meem
_______________________________________________
networking-discuss mailing list
[email protected]
