please find attached file x509-sn_is__.patch.gz with latest changes.
======================================================
On 18 Jul 2003 Aleksey Sanin wrote:
<SNIP>
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.
fixed
2) xmlSecOpenSSLASN1IntegerWrite() function: the ASN1_INTEGER_to_BN()
may return NULL. Instead of assert you should use if() to check the result.
fixed
Also I wonder why do you use '_xxx' variable? Why do you need '_'?
now method args are without leadnig '_'
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.
fixed
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.
removed
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.
new method xmlSecGetX509NodeWriter
1) Create separate methods like:
xmlSecOpenSSLX509CertificateNodeWrite
xmlSecOpenSSLX509SubjectNameNodeWrite
xmlSecOpenSSLX509IssuerSerialNodeWrite
xmlSecOpenSSLX509SKINodeWrite
implemented
2) Call one these methods from the for() loop in xmlSecOpenSSLKeyDataX509XmlWrite()
for each cert in the keys data.
implemented call for selected method
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.
implemented
======================================================
On 24 Jul 2003 Aleksey Sanin wrote:
<SNIP>
1) I don't like the way you implemented the "empty" check in *Read() functions.
IMHO, this is a bad coding style to repeat the same code again and again.
Probably a small internal static function
int xmlSecOpenSSLX509IsEmpty(xmlChar*)
would be better :)
:-) yes of course - new method xmlSecIsWhiteSpaceString
Also I am not sure I understand why you put "XXX" comments around it. Seems
useless to me.
now code has real comments instead of XXX and calls for xmlSecIsWhiteSpaceString
2) You are using the figure brackets to mark block of code all the time
(I meant the "write Issuer Name" block in the example bellow):
+ if(cur == NULL) {
+ cur = xmlSecAddChild(node, xmlSecNodeX509IssuerSerial, xmlSecDSigNs);
+ ....
+ }
+
+ { /*write Issuer Name*/
+ for(node_in = xmlSecGetNextElementNode(cur->children);
+ ....
+ }
Please don't do this. It makes code difficult to read.
now code is without "figure brackets", but I dont agree that code in brackets is difficult to read. See comments at end of mail
3) In xmlSecOpenSSLX509NameWrite() function I wonder if there is a way to
print name to a buffer, not memory BIO. Mallocs might be expensive :( But I guess
the answer is "no".
somebody experienced in BIO and libxml2 architecture might can write method BIO_xmlNode for reading/writind data direct from/to xml node content, but I'm not that person :-(
4) Which OpenSSL version do you use? I wonder if this new code works with OpenSSL 0.9.6.
:-[ code is compiled with OpenSSL 0.9.6e and 0.9.7b
====================================================== Miscellaneous:
In a method we can put part of code in "figure brackets" to make source more readable.
Suppose we have method:
int method() {
int A, .../*5 variables to compute A*/;
int B, .../*variables to compute B which cannot be shared with already defined*/;
/*code to compute A*/ ......./*N lines code */ /*code to compute B*/ ......./*K lines code */
return(...); }
My opinion is to rewrite like this.
int method() {
int A;
int B; { /*code to compute A*/
definition of 5 necessary variables to compute A
..../*N lines code */
} { /*code to compute B*/
definition of variables necessary to compute B
..../*K lines code */
}return(...); }
Code in brackets is complete and I can find easy start/end of "code-block". This style has other advantages as easy to modify, low stack requrenment.
Best regards Roumen Petrov
x509-sn_is__.patch.gz
Description: application/gzip
