Hi Paul, Thank you for the feedback. Please see my comments intersperced below.
Paul Cunningham wrote: > Hi, > > Please see below for my comments from my very quick skip through ... > > Paul > > Huie-Ying Lee wrote: >> >> Please help review code for the pam_pkcs11 project which is to be >> integrated into SFWNV consolication. >> >> The webrev is located at: http://cr.opensolaris.org/~hylee/pam_pkcs11/ > > === Start of Comments === > > 1. usr/src/lib/pam_pkcs11/install-pampkcs11 > & usr/src/lib/pam_pkcs11/install-pampkcs11-64 > You don't need the 'mkdir -p's as the directories should be in > Targetdirs and so created already. OK. Will fix. As the directories should be in Targetdirs, I think I should also add the following directories to the usr/src/Targetdirs file. ------- Targetdirs ------- 70a71,73 > /etc/security/pam_pkcs11 \ > /etc/security/pam_pkcs11/cacerts \ > /etc/security/pam_pkcs11/crls \ 736a740 > /usr/share/doc/pam_pkcs11 \ > 2. usr/src/lib/pam_pkcs11/Makefile.sfw > For clarity it might be nice to include '--prefix=' on > the 'configure's, but that should down using the predefined > value for '--prefix=' from Makefile.master, see the following > as an example > http://cr.opensolaris.org/~rayx/erlang/webrev/usr/src/cmd/erlang/Makefile.sfw.html > > > OK. Will fix. > 3. usr/src/pkgdefs/SUNWpampkcs11u/depend > Move the copyright lines to after the 'CDDL HEADER END' header > > This also looks like the default 'depend', shouldn't it include > your root pkg at minimum. > OK. Will fix both problems. > 4. man pages > Should there be any 'man' pages with this package ? > There are 3 man pages for this project: pam_pkcs11.5 pkcs11_inspect.1 pklogin_finder.1 They will be delivered in the SUNWman package by the man page writer seperately. For more information, please refer to CR6671156 and CR6671162. The reason that they are shipped seperately is because we enhanced the man pages quite a bit for it to be more helpful. > 5. Every thing else looks okay to me > > === End of Comments ===== I will update the sources to incorperate your suggestions and post an updated webrev later. Thanks, Huie-Ying
