<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=UTF-8">
<title></title>
</head>
<body text="#000000" bgcolor="#ffffff">
<meta http-equiv="Content-Type" content="text/html;charset=UTF-8">
<title></title>
Hi, Roumen!<br>
<br>
I have looked at your new patch and I have few comments:<br>
0) It seems that xmlSecOpenSSLKeyDataX509XmlWrite() function<br>
now writes subject, serial or full certificate only for the first
certificate<br>
in the xmlSecKey. All other certificates are written "in-full".<br>
This seems wrong to me. Yo don't know which certificate will be the
<br>
"first" one. I am not sure I understand why you don't want to do the<br>
same for all certs. <br>
I thought that the plan was: <br>
- Read X509Data node and create a bits mask of its children<br>
(cert, subject, serial, ski, crl).<br>
- If mask is 0 (no children) then set cert and crl bits to
simulate<br>
current behaiviour (write certs and crls in empty X509Data
node).<br>
- Remove X509Data node content.<br>
- Walk thru the list of certificates and write cert and/or
subject and/or<br>
serial and/or ski according to bits mask.<br>
- If crls bit is set walk thru the list of crls and write
them out.<br>
This seems more natural to me than "special case" the first cert.<br>
<br>
1) I don't like the way you implemented the "empty" check in
*Read() functions.<br>
IMHO, this is a bad coding style to repeat the same code again and
again.<br>
Probably a small internal static function <br>
int xmlSecOpenSSLX509IsEmpty(xmlChar*)<br>
would be better :)<br>
Also I am not sure I understand why you put "XXX" comments around
it. Seems<br>
useless to me.<br>
<br>
2) You are using the figure brackets to mark block of code all the
time<br>
(I meant the "write Issuer Name" block in the example bellow):<br>
<br>
+ if(cur == NULL) {<br>
+ cur = xmlSecAddChild(node, xmlSecNodeX509IssuerSerial,
xmlSecDSigNs);<br>
+ ....<br>
+ }<br>
+<br>
+ { /*write Issuer Name*/<br>
+ for(node_in = xmlSecGetNextElementNode(cur->children);<br>
+ ....<br>
+ }<br>
<br>
Please don't do this. It makes code difficult to read. <br>
<br>
3) In xmlSecOpenSSLX509NameWrite() function I wonder if there is a
way to <br>
print name to a buffer, not memory BIO. Mallocs might be expensive
:( But I guess<br>
the answer is "no".<br>
<br>
4) Which OpenSSL version do you use? I wonder if this new code
works with OpenSSL 0.9.6.<br>
<br>
<br>
Aleksey<br>
<br>
<br>
</body>
</html>