incubator-ooo-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject [Bug 120040] There is a memory leak in function SvBaseLink::~SvBaseLink()
Date Wed, 20 Jun 2012 18:39:02 GMT
https://issues.apache.org/ooo/show_bug.cgi?id=120040

--- Comment #5 from ChaoHuang <chao.dev.h@gmail.com> ---
(In reply to comment #3)
> ALG: SvBaseLink has three constructors, and only in two of them pImpl gets
> allocated and thus initialized. Thus, in the 3rd constructor, pImpl will
> point to a random address. Freeing it in the destructor could crash the
> office.
> 
> A minimal fix would have to init pImpl to zero and check before
> construction. But even then,  the implementation makes use of pImpl without
> tests, so maybe in the 3rd constructor it should also be allocated. All in
> all, the whole class would need more rework, all members initialized in all
> constructors, and pImpl decided for 3rd constructor.

>From my point of view, pImpl is a fundamental member in class SvBaseLink, which
should be initialized in every constructor. There is no interface to set the
value for pImpl. What's more, there is no checking for pImpl before use it, and
pImpl is initialized in the first two constructors. I think that it should be
also initialized in the third constructor.

Another concern I want to say is that it should be well to add an assert in the
 beginning of the function than in the end.

I prefer code style
-------------------------------------------------------------
void SvBaseLink::SetLinkManager( LinkManager* _pMgr )
{
    OSL_ENSURE(pImpl!=NULL, "No pImpl (!)");
    pImpl->m_pLinkMgr = _pMgr;
}
-------------------------------------------------------------

to

-------------------------------------------------------------
void SvBaseLink::SetLinkManager( LinkManager* _pMgr )
{
    if(pImpl)
    {
        pImpl->m_pLinkMgr = _pMgr;
    }
    else
    {
        OSL_ENSURE(false, "No pImpl (!)");
    }
}
-------------------------------------------------------------

-- 
You are receiving this mail because:
You are the assignee for the bug.

Mime
View raw message