Re: [xmlsec] core methods for write of and

[Aleksey Sanin]

Hi, Roumen! I have looked at your new patch and I have few comments:     0) It seems that xmlSecOpenSSLKeyDataX509XmlWrite() function     now writes subject, serial or full certificate only for the first certificate     in the xmlSecKey. All other certificates are written "in-full".     This seems wrong to me. Yo don't know which certificate will be the     "first" one. I am not sure I understand why you don't want to do the     same for all certs.     I thought that the plan was:             - Read X509Data node and create a bits mask of its children           (cert, subject, serial, ski, crl).           - If mask is 0 (no children) then set cert and crl bits to simulate           current behaiviour (write certs and crls in empty X509Data node).           - Remove X509Data node content.           - Walk thru the list of certificates and write cert and/or subject and/or           serial and/or ski according to bits mask.           - If crls bit is set walk thru the list of crls and write them out.     This seems more natural to me than "special case" the first cert.     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 :)     Also I am not sure I understand why you put "XXX" comments around it. Seems     useless to me.     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.     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".     4) Which OpenSSL version do you use? I wonder if this new code works with OpenSSL 0.9.6. Aleksey