[libvirt] [PATCH 02/14] esx: Address resource leak found by Coverity

[Matthias Bolte]

2013/1/10 Eric Blake <eblake at redhat.com>:
> On 01/09/2013 07:54 AM, John Ferlan wrote:
>> Because result was used to determine whether or not to free 'priv'
>> resources Coverity tagged the code as having a resource leak. This
>> change addresses that concern.
>> ---
>>  src/esx/esx_driver.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
>> index 1366c81..9befa38 100644
>> --- a/src/esx/esx_driver.c
>> +++ b/src/esx/esx_driver.c
>> @@ -998,6 +998,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
>>          virReportOOMError();
>>          goto cleanup;
>>      }
>> +    conn->privateData = priv;
>>
>>      if (esxUtil_ParseUri(&priv->parsedUri, conn->uri) < 0) {
>>          goto cleanup;
>
> I
>
>> @@ -1008,8 +1009,6 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
>>      priv->supportsLongMode = esxVI_Boolean_Undefined;
>>      priv->usedCpuTimeCounterId = -1;
>>
>> -    conn->privateData = priv;
>> -
>>      /*
>>       * Set the port dependent on the transport protocol if no port is
>>       * specified. This allows us to rely on the port parameter being
>> @@ -1104,9 +1103,10 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
>>      result = VIR_DRV_OPEN_SUCCESS;
>>
>>    cleanup:
>> -    if (result == VIR_DRV_OPEN_ERROR) {
>> +    if (result == VIR_DRV_OPEN_ERROR)
>> +        conn->privateData = NULL;
>> +    if (priv && !conn->privateData)
>>          esxFreePrivate(&priv);
>> -    }
>
> This feels a bit complex;

I agree.

> I had to go read esxFreePrivate() to make sure
> it would behave if called on a partial object.  Would it be any easier
> to delay the assignment to conn->privateData, and use transfer of
> ownership semantics, so that the cleanup is unconditional?
>
>     conn->privateData = priv;
>     priv = NULL;
>     result = VIR_DRV_OPEN_SUCCESS;
> cleanup:
>     esxFreePrivate(&priv);

No! This cannot be done that easily, see commit
b126715a48cd0cbe32ec6468c267cd8cf2961c55 "esx: Fix segfault in
esxConnectToHost".

We cannot move conn->privateData = priv; further down with the current
code, because it is already used in between. But I like the simplicity
of your suggestion.

I'll me come up with a patch for this later today.

-- 
Matthias Bolte
http://photron.blogspot.com