Aleksey Sanin wrote:
> > > Please check http://roumenpetrov.info/tmp/xmlsec/ for the files. > > About patch: > > - please review new methods - they are release candidates; > > - all other is very early release, even before alpha version ;-). > > 0) I absolutely do not like the code in > xmlSecOpenSSLKeyDataX509XmlWrite() > function. As I explained before using static variable is wrong idea and > using numeric constants instead of defines is really bad code style. > 1) xmlSecOpenSSLX509NameWrite() function: xmlMalloc may fail. You need > to check that returned pointer is not NULL and return an error if it's > the case. > 2) xmlSecOpenSSLASN1IntegerWrite() function: the ASN1_INTEGER_to_BN() > may return NULL. Instead of assert you should use if() to check the > result. > Also I wonder why do you use '_xxx' variable? Why do you need '_'? > 2) xmlSecOpenSSLASN1IntegerWrite() function: The function returns > xmlChar* allocated using OpenSSL function BN_bn2dec(). This is wrong! > xmlChar* is assumed to be allocated with one of LibXML2 malloc functions > and is freed with xmlFree. If there is a different memory callbacks > initialized > in LibXML2 this code would crash. > 3) testDSig.sh: I don't see reasons to modify existing tests. The right > way is to add > new tests to the suite to test new functionality. > > > > IMHO, the better approach would be: > 0) At the very beggining of the xmlSecOpenSSLKeyDataX509XmlWrite() > function you read the <X509Data/> node content and determine what do you > want > to write (certs, subject names, ...) based on the content of <X509Data/> > node > and the xmlSecKeyInfoCtx flags. > 1) Create separate methods like: > xmlSecOpenSSLX509CertificateNodeWrite > xmlSecOpenSSLX509SubjectNameNodeWrite > xmlSecOpenSSLX509IssuerSerialNodeWrite > xmlSecOpenSSLX509SKINodeWrite > 2) Call one these methods from the for() loop in > xmlSecOpenSSLKeyDataX509XmlWrite() > for each cert in the keys data. > 3) Determine if you want to write CRLs > (XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_WRITE CRLS > flag in the xmlSecKeyInfoCtx and call the new > xmlSecOpenSSLX509CRLNodeWrite > function for each CRL in xmlSecOpenSSLX509Data if needed. > > > > Tej, I wonder if it would be possible to mirror this in NSS code. What > do you think? The scheme you outlined makes perfect sense. This is easy to mirror in the NSS port. I'll get to it in the near future.. :) (i've just started working on another project ...) -Tej _______________________________________________ xmlsec mailing list [EMAIL PROTECTED] http://www.aleksey.com/mailman/listinfo/xmlsec
