> dladm.c:
>
> * 5503: Seems like "/etc/dladm/datalink.conf" should just be in the
> format string itself.
>
I intentionally did that because the indent does not look good this way.
Anyway, I changed
as you requested.
> dlmgmt_db.c:
>
> * 374, 378: This appears to be an unrelated bugfix.
>
Filed 6671333.
> dlmgmt_main.c:
>
> * 101, 233: Comment exposes implementation -- would prefer to
> refer to the macro name so that it won't get stale if the macro
> value gets updated. (Also, why do we have dlmgmt_door_file[]
> when it's just DLMGMT_DOOR?)
>
Okay. Changed.
> * 103: Why specify O_EXCL but ignore EEXIST? O_CREAT|O_RDONLY
> seems more intuitive to me.
>
> * 103: Why S_IWRITE? (And I think S_IRUSR is the more
> standardized form of S_IREAD.)
>
Apparently, write permission is required in order to do fattach(). I've changed
to
O_CREAT|O_RDWR and S_IRUSR|S_IWUSR.
> * 225: Extra trailing ";"
>
Removed.
> * 249: s/the root privileges/root privilege/ -- but "root" isn't a
> privilege. What privilege is actually needed?
>
"ALL" privilege is required. I've fixed the comment.
> * 259: The old code provided a helpful error message when dlmgmtd
> was run with insufficient privileges. Seems like that's gone (?)
>
Line 358 will prompt the error message for this case.
> dls.h:
>
> * 110: Should have a comment explaining its purpose.
>
Added.
> * 111: Add blank line to separate it from upcall commands (and
> while you're here, /s/command./commands./
>
Removed.
Again, the webrev is updated at:
http://cr.opensolaris.org/~yun/webrev_dlmgmtd/
Thanks
- Cathy
