[kmf-discuss] KMF code review (part II.)

[Huie-Ying Lee]

Wyllys Ingersoll wrote:

> Thanks for the comments so far.   The webrev will be updated shortly.
> 
> -Wyllys
> 

Here is my code review comments for the updated webrev.

signcsr.c: yn_to_int() (line 50-64)

   Do we really need the yn_to_int() function ?
   I think we can reuse the yesno() function in common.c for the same purpose.

signcsr.c: write_cert_file()

   This function is not needed, because the caller can use the existing API
   kmf_create_cert_file(const KMF_DATA *, KMF_ENCODE_FORMAT, char *);

kmfapi.h: line 327

   Why change kmf_select_token() to be a public API?   

kmfapiP.h: line 354

   Nit; Consider rename eku_present() to is_eku_present(), because I think
   is_eku_present() sounds better. 8-)

kmftypes.h: line 331

   Not needed.  This line is really a duplicate of line 329.

csrcrlop.c, kmf_decode_csr()

   line 521, does the csrdata has enough space ?  Should it be checked ?

certop.c:

   The kmf_sign_data() API needs to be updated to check the key usage policy
   as well. 

pem_encode.c: get_line(), line 474, 479 and 482

   Nit; it might be better to get rid of the unnecessary variable - "len".
   This function can just return "i" in the end.

pkcs11_spi.c: line 2737 and 2738

   Nit; It might be better if combining these 2 lines into 1 line, such as 
        if (cred != NULL && cred->credlen > 0)


Huie-Ying