Looks good overall, some minor comments:
linkprop.c:
1945 if (prop_val == NULL) {
1946 if ((status = i_dladm_ioctl(fd, DLDIOCGETPROP, dip,
1947 dsize)) < 0) {
1948 free(dip);
1949 return (dladm_errno2status(errno));
fd leak.
1966 if ((status = i_dladm_ioctl(fd, DLDIOCSETPROP, dip, dsize))
< 0) {
1967 free(dip);
1968 return (dladm_errno2status(errno));
1969 }
If i_dladm_ioctl() returns value from ioctl(), it probably shouldn't be
assigned to dladm_status_t status?
mac_ndd.c:
196 mp->b_cont = allocb(len, BPRI_HI); /* XXX allocate >
len? */
XXX means something's wrong here?
309 mp1 = allocb(avail, BPRI_HI); /* the returned buffer */
...
403 status = mip->mi_callbacks->mc_getprop(mip->mi_driver,
priv_name,
404 DLD_PROP_PRIVATE, 0, avail, mp1->b_rptr);
...
411 size_out += strnlen((const char *)mp1->b_rptr, avail);
412 valp += size_out;
413 *valp++ = '\0'; /* need \0\0 */
414 *valp++ = '\0';
If driver returns a string of length avail-1, wouldn't line 414 overflow
the buffer?
mac.c, bge.c:
The way we use bcopy() to copy 8-bit and 32-bit values should look
pretty funny to someone reviewing the code for the first time :)
mac.c:
1054 if ((cmd == ND_GET && (mip->mi_callbacks->mc_callbacks &
MC_GETPROP)) ||
1055 (cmd == ND_SET && (mip->mi_callbacks->mc_callbacks &
MC_SETPROP))) {
1056 /*
1057 * Brussels driver. If ndd props were registered,
call them.
I'm not sure the project codename will be remembered for long and make
sense to most people. I think if we take out "Brussels driver", the rest
of the comment won't lose much, it's pretty obvious code.
1884 mpriv = mip->mi_priv_prop;
1885 kmem_free(mpriv, mip->mi_npriv_prop * sizeof (*mpriv));
It took me a few seconds to register that "n". Variable names should
differ in more than one character, imho. I'm not feeling strongly about
this - up to you.
-Artem
