[libvirt] [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. 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(-) 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; } 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: @@ -2327,7 +2328,7 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, if (*virtualMachine == NULL) { if (occurrence == esxVI_Occurrence_OptionalItem) { - return 0; + goto success; } else { ESX_VI_ERROR(VIR_ERR_NO_DOMAIN, _("Could not find domain with name '%s'"), name); @@ -2335,6 +2336,7 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx, const char *name, } } + success: result = 0; cleanup: @@ -2890,7 +2892,7 @@ esxVI_LookupCurrentSnapshotTree if (currentSnapshot == NULL) { if (occurrence == esxVI_Occurrence_OptionalItem) { - return 0; + goto success; } else { ESX_VI_ERROR(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", _("Domain has no current snapshot")); @@ -2911,6 +2913,7 @@ esxVI_LookupCurrentSnapshotTree goto cleanup; } + success: result = 0; cleanup: @@ -3050,9 +3053,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, /* Interpret search result */ if (searchResults->file == NULL) { if (occurrence == esxVI_Occurrence_OptionalItem) { - result = 0; - - goto cleanup; + goto success; } else { ESX_VI_ERROR(VIR_ERR_NO_STORAGE_VOL, _("No storage volume with key or path '%s'"), @@ -3064,6 +3065,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, *fileInfo = searchResults->file; searchResults->file = NULL; + success: result = 0; cleanup: -- 1.7.0.4
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. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2010/8/10 Eric Blake <eblake@redhat.com>:
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
On 08/15/2010 12:34 PM, Matthias Bolte wrote:
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.
ACK to this version. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2010/8/16 Eric Blake <eblake@redhat.com>:
On 08/15/2010 12:34 PM, Matthias Bolte wrote:
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.
ACK to this version.
Thanks, pushed. Matthias
participants (2)
-
Eric Blake -
Matthias Bolte