Re: [PATCH] esx: Simplify goto usage (part 1/2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2010/5/26 Eric Blake <eblake@xxxxxxxxxx>:
> 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]