2010/8/23 Eduardo Otubo <otubo@xxxxxxxxxxxxxxxxxx>: > I fixed the way I check for invalid values and changed the way I report > errors (from VIR_ERROR0 to PHYP_ERROR). I'll change the VIR_WARNs in > another different patch. Okay, that's fine. > --- > src/phyp/phyp_driver.c | 23 +++++++++++++++++++++++ > 1 files changed, 23 insertions(+), 0 deletions(-) > > diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c > index 251111d..a74eedf 100644 > --- a/src/phyp/phyp_driver.c > +++ b/src/phyp/phyp_driver.c > @@ -3701,6 +3701,29 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) > int exit_status = 0; > virBuffer buf = VIR_BUFFER_INITIALIZER; > > + if (def->memory > 0) { > + PHYP_ERROR(VIR_ERR_XML_ERROR,"%s", > + _("Field \"<memory>\" on the domain XML file is missing or has " > + "invalid value.")); > + goto err; > + } > + > + if (def->maxmem > 0) { > + PHYP_ERROR(VIR_ERR_XML_ERROR,"%s", > + _("Field \"<currentMemory>\" on the domain XML file is missing or" > + " has invalid value.")); > + goto err; > + } Sorry for confusing you about "comparing > 0", now the check is wrong here and you report an error when the input is actually valid. Please change this back to "if (!def->memory)". > + if (def->ndisks > 0) { > + if (!def->disks[0]->src) { > + PHYP_ERROR(VIR_ERR_XML_ERROR,"%s", > + _("Field \"<src>\" under \"<disk>\" on the domain XML file is " > + "missing.")); > + goto err; > + } > + } Okay, no potential segfault here anymore. But a few lines below you use def->disks[0]->src in a virBufferVSprintf call, without checking if the first disk element is there. So you either need to make the first disk element mandatory by checking for def->ndisks being 0 and reporting an error in that case, or by altering the virBufferVSprintf call to only output the virtual_scsi_adapters=%s part when the domain XML contains a disk element. Matthias -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list