Re: [libvirt] [PATCH] esx: Simplify goto usage (part 1/2)
2010/5/26 Eric Blake <eblake(a)redhat.com>:
On 05/26/2010 10:30 AM, Matthias Bolte wrote:
> Eliminate almost all backward jumps by replacing this common pattern:
> @@ -1786,21 +1733,17 @@ esxDomainGetInfo(virDomainPtr domain,
virDomainInfoPtr info)
> esxVI_LookupVirtualMachineByUuid(priv->host, domain->uuid,
> propertyNameList, &virtualMachine,
> esxVI_Occurrence_RequiredItem) < 0) {
> - goto failure;
> + goto cleanup;
> }
>
> info->state = VIR_DOMAIN_NOSTATE;
> - info->maxMem = 0;
> - info->memory = 0;
> - info->nrVirtCpu = 0;
> - info->cpuTime = 0; /* FIXME */
I didn't quite follow this deletion just from the patch itself; are
these values not valid if info->state is VIR_DOMAIN_NOSTATE?
These values get all set to 0, but I change it to a memset(info, 0,
sizeof (*info)); in the hunk before this one. VIR_DOMAIN_NOSTATE is
actually 0 too, but I left it there for clarity.
> @@ -2219,6 +2158,10 @@ esxDomainDumpXML(virDomainPtr domain, int
flags)
> }
>
> cleanup:
> + if (url == NULL) {
> + virBufferFreeAndReset(&buffer);
> + }
> +
> esxVI_String_Free(&propertyNameList);
> esxVI_ObjectContent_Free(&virtualMachine);
> VIR_FREE(datastoreName);
> @@ -2229,12 +2172,6 @@ esxDomainDumpXML(virDomainPtr domain, int flags)
> virDomainDefFree(def);
>
> return xml;
> -
> - failure:
> - virBufferFreeAndReset(&buffer);
> - VIR_FREE(xml);
Did we lose a VIR_FREE(xml) on this conversion? Or can we guarantee
that xml is NULL if url is NULL?
The VIR_FREE was useless at this point, because xml =
virDomainDefFormat(def, flags); is the last line directly above the
cleanup label, therefore there was no goto failure in between that
could lead to a call to VIR_FREE with xml != NULL.
ACK, assuming you can either answer those questions or tweak the code to
fix them.
Thanks.
Matthias