[libvirt] [PATCH] esx: Avoid Coverity warning about resource leak in esxOpen

[John Ferlan]

On 01/10/2013 05:27 PM, Eric Blake wrote:
> On 01/10/2013 02:47 PM, Matthias Bolte wrote:
>> Commit 4445e16bfa8056980ac643fabf17186f9e685925 changed the signature
>> of esxConnectToHost and esxConnectToVCenter by replacing the esxPrivate
>> pointer with virConnectPtr. The esxPrivate pointer was then retrieved
>> again from virConnectPtr's privateData. This resulted in a NULL pointer
>> dereference, because the privateData pointer was not yet initialized at
>> the point where esxConnectToHost and esxConnectToVCenter are called.
>>
>> This was fixed in commit b126715a48cd0cbe32ec6468c267cd8cf2961c55 that
>> moved the initialization of privateData before the problematic calls.
>>
>> Coverity tagged the esxPrivate pointer a potentially leaked because of
>> the conditional free call. But this is a false positive, there is not
>> actual leak.
>>
>> Avoid this warning from Coverity by making the call to esxFreePrivate
>> unconditional and changing esxConnectToHost and esxConnectToVCenter back
>> to take a esxPrivate pointer directly. This allows to assign esxPrivate
>> to the virConnectPtr's privateData pointer as one of the last steps in
>> esxOpen making it more obvious that it is not initialized during the
>> earlier steps of esxOpen.
>> ---
>>
> 
> ACK.
> 

Still get a Coverity error, but I think it has something to do with the
VIR_FREE(*priv); done in the esxFreePrivate(). I'm beginning to believe
it's a Coverity "issue".  I have no idea why what I had done previously
was able to bypass the error.  I'm trying to figure out the right
mechanism to ask in the Coverity world.

I've been chasing issues in the 'esx_vi.c' and 'esx_vi_types.c' code
today which is why I note this.  I was able to put together some sample
code that removes all the macros and it ended up being very similar to
this code ironically.

>> This patch is meant as a replacement this patch:
>>
>> https://www.redhat.com/archives/libvir-list/2013-January/msg00530.html
> 
> John, thanks again for pointing out the Coverity warning, and doing a
> first pass analysis, even if we didn't end up using your patch.  One of
> the nice things about open source is that by making the problems public,
> we can often come up with even better patches.  And Matthias, thanks for
> jumping in to improve the code you originally wrote.
> 
> 
I'm perfectly fine with someone else making changes.

> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>