[libvirt] [PATCHv2] PHYP: Checking for NULL values when building new guest

[Matthias Bolte]

2010/8/23 Eduardo Otubo <otubo at linux.vnet.ibm.com>:
> 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