Tej,

It took less time than I expected to look at your changes :) Almost everything looks
great except few minor things listed bellow. I have checked in your patch into
the same XMLSEC_NSS_030714 with only one change (item 0) bellow).
However, there are a couple items (1) and 2)) that needs to be resolved
before we can put this code into the trunk.


BTW, I can't run the xmlsec-nss tests because of certutiland I could not test xmlsec-nss
with valgrind :(


Aleksey



0) I have moved the NSS DB reset code from test*.sh scripts to the top level
Makefile.am. No need to repeat it again and again. This allowed me to remove
xmlsec-config option from the test*.sh scripts. Also I don't think that we want
to pass crypto_config to these scripts because nobody else should use it
anyway (it is also removed).
1) As I mentioned before, I don't have "cryptoutil" installed. Thus other users
might miss it too :) We need to find a way to avoid using this tool in the tests scripts.
Is it possible to initializa the nss db from xmlSecNssAppInit() instead (if config is not
NULL and NSS DB does not exist there, create it)?
2) xmlSecNssAppPkcs12Load(): why do you do these initializations in this function?
PORT_SetUCS2_ASCIIConversionFunction(xmlSecNssAppAscii2UCS2Conv);
SEC_PKCS12EnableCipher(...)
SEC_PKCS12EnableCipher(...)
It seems to me that it's a wrong idea. These initializations are global. The function
can be called many times from multiple threads. IMO, these initialization calls
need to be done once in applicaiton init function (xmlSecNssAppInit?).
Please take a look at it.
3) keysstore.c: I wonder why do you need to have simple keys store in default nss
keys manager? Why you could not use only NSS keydb?
4) warnings about __CERT_NewTempCertificate function:
src/nss/app.c:847: warning: implicit declaration of function `__CERT_NewTempCertificate'
x509.c:1494: warning: implicit declaration of function `__CERT_NewTempCertificate'
Is it worth it to put a declaration somewhere in xmlsec-nss headers if it does not exist in NSS?




_______________________________________________
xmlsec mailing list
[EMAIL PROTECTED]
http://www.aleksey.com/mailman/listinfo/xmlsec

Reply via email to