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.

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

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.

4. man pages
    Should there be any 'man' pages with this package ?

5. Every thing else looks okay to me

=== End of Comments =====

-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products

Reply via email to