2010/8/10 Eric Blake <eblake@xxxxxxxxxx>:
> On 08/08/2010 02:56 PM, Matthias Bolte wrote:
>> In case an optional object cannot be found the lookup function is
>> left early and the cleanup code is not executed. Add a success label
>> and goto instead of an early return.
>>
>> This pattern occurs in some other functions too.
>> ---
>> src/esx/esx_vi.c | 14 ++++++++------
>> 1 files changed, 8 insertions(+), 6 deletions(-)
>
> Definitely a leak to plug, but:
>
>>
>> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
>> index c6b2b63..b064b11 100644
>> --- a/src/esx/esx_vi.c
>> +++ b/src/esx/esx_vi.c
>> @@ -2254,7 +2254,7 @@ esxVI_LookupVirtualMachineByUuid(esxVI_Context *ctx, const unsigned char *uuid,
>>
>> if (managedObjectReference == NULL) {
>> if (occurrence == esxVI_Occurrence_OptionalItem) {
>> - return 0;
>> + goto success;
>
> Why not use the idiom:
>
> result = 0;
> goto cleanup;
>
>> } else {
>> ESX_VI_ERROR(VIR_ERR_NO_DOMAIN,
>> _("Could not find domain with UUID '%s'"),
>> @@ -2270,6 +2270,7 @@ esxVI_LookupVirtualMachineByUuid(esxVI_Context *ctx, const unsigned char *uuid,
>> goto cleanup;
>> }
>>
>> + success:
>> result = 0;
>>
>> cleanup:
>
> such that you only have to have one label, instead of adding a new
> label? All three instances follow the same pattern.
>
Here's v2 attached. I also removed two already existing success labels.
Matthias
From e98dd282cbe6d92accffb6cde6c9be973dfe10a6 Mon Sep 17 00:00:00 2001
From: Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>
Date: Sun, 8 Aug 2010 21:32:29 +0200
Subject: [PATCH] esx: Fix memory leak when looking up an non-existing domain by name
In case an optional object cannot be found the lookup function is
left early and the cleanup code is not executed.
This pattern occurs in some other functions too.
---
src/esx/esx_vi.c | 24 ++++++++++++++++--------
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 55c5246..3773a5f 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -2254,7 +2254,9 @@ esxVI_LookupVirtualMachineByUuid(esxVI_Context *ctx, const unsigned char *uuid,
if (managedObjectReference == NULL) {
if (occurrence == esxVI_Occurrence_OptionalItem) {
- return 0;
+ result = 0;
+
+ goto cleanup;
} else {
ESX_VI_ERROR(VIR_ERR_NO_DOMAIN,
_("Could not find domain with UUID '%s'"),
@@ -2327,7 +2329,9 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name,
if (*virtualMachine == NULL) {
if (occurrence == esxVI_Occurrence_OptionalItem) {
- return 0;
+ result = 0;
+
+ goto cleanup;
} else {
ESX_VI_ERROR(VIR_ERR_NO_DOMAIN,
_("Could not find domain with name '%s'"), name);
@@ -2454,8 +2458,10 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name,
goto cleanup;
}
- // Found datastore with matching name
- goto success;
+ /* Found datastore with matching name */
+ result = 0;
+
+ goto cleanup;
}
}
@@ -2465,7 +2471,6 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name,
goto cleanup;
}
- success:
result = 0;
cleanup:
@@ -2540,7 +2545,9 @@ esxVI_LookupDatastoreByAbsolutePath(esxVI_Context *ctx,
}
/* Found datastore with matching mount path */
- goto success;
+ result = 0;
+
+ goto cleanup;
}
}
}
@@ -2552,7 +2559,6 @@ esxVI_LookupDatastoreByAbsolutePath(esxVI_Context *ctx,
goto cleanup;
}
- success:
result = 0;
cleanup:
@@ -2890,7 +2896,9 @@ esxVI_LookupCurrentSnapshotTree
if (currentSnapshot == NULL) {
if (occurrence == esxVI_Occurrence_OptionalItem) {
- return 0;
+ result = 0;
+
+ goto cleanup;
} else {
ESX_VI_ERROR(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s",
_("Domain has no current snapshot"));
--
1.7.0.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list