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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list