Re: [libvirt] [PATCH] esx: Avoid Coverity warning about resource leak in esxOpen
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list