On 2/23/22 04:41, Huang, Haibin wrote:
I accept all comments. But I didn't your point for this comment .
+static void +virQEMUCapsFormatSGXInfo(virQEMUCaps *qemuCaps, virBuffer *buf) { + virSGXCapabilityPtr sgx = +virQEMUCapsGetSGXCapabilities(qemuCaps); + + virBufferAddLit(buf, "<sgx>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : + "no");
... which is in contradiction with the way we format it. [Haibin] sorry, I don't get your point, can you give me the details
Does you mean that change virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : "no"); to virBufferAsprintf(buf, "<flc>%u</flc>\n", sgx->flc);
No. You can keep this particual virBufferAsprintf() as is. What I have problem with is the parsing side. So if this virQEMUCapsFormatSGXInfo() function is called, if produces the following XML: <sgx> <flc>no</flc> <epc_size>1024</epc_size> </sgx> (I've made the values up) This corresponds to: sgx->flc = false; sgx->epc_size = 1024; But when the XML is parsed, in virQEMUCapsParseSGXInfo(), it would se the following values: sgx->flc = true; sgx->epc_size = 2014; Notice the change in ->flc? This is because virXPathBoolean("boolean(./sgx/flc)") does not evaluate the value of <flc/> element, but only whether the xpath exists. Therefore, even if flc's element value is "no", it does exist and as such is evaluated to true. Hope this cleans things up. Michal