2013/1/10 Eric Blake <eblake@xxxxxxxxxx>:
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list