incubator-ooo-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject [Bug 120106] There is a memory leak in function SwHTMLWriter::CollectFlyFrms
Date Fri, 29 Jun 2012 04:08:01 GMT
https://issues.apache.org/ooo/show_bug.cgi?id=120106

--- Comment #5 from ChaoHuang <chao.dev.h@gmail.com> ---
(In reply to comment #4)
> Created attachment 78515 [details]
> Extended patch
> 
> ALG->ChaoHuang: I replaced the old SV_PTR_* stuff with stl usage and secured
> all usages, please have a look. Thanks!

There are three places to call function "SwDoc::GetAllFlyFmts"
  1)SwXParaFrameEnumeration::SwXParaFrameEnumeration    in unoobj2.cxx  
  2)sw.util.GetFrames    in writerhelper.cxx
  3)SwHTMLWriter::CollectFlyFrms()    in htmlfly.cxx

There is always a definition for a local SwPosFlyFrms object before calling
"SwDoc::GetAllFlyFmts". Then the reference of the local SwPosFlyFrms object
will be passed into "SwDoc::GetAllFlyFmts" to fill with the result. So it is OK
to return a local SwPosFlyFrms obj in function "SwDoc::GetAllFlyFmts" and to
use it to create a SwPosFlyFrms  obj. So I agree to change the header for
function "SwDoc::GetAllFlyFmts" 
from
      void SwDoc::GetAllFlyFmts( SwPosFlyFrms& rPosFlyFmts,
                                 const SwPaM* pCmpRange, 
                                 sal_Bool bDrawAlso ) const
to
      SwPosFlyFrms SwDoc::GetAllFlyFmts( const SwPaM* pCmpRange, 
                                         sal_Bool bDrawAlso ) const

I have one question about function "SwPosFlyFrmCmp::operator()" in new file
According to the code in previous function "SwPosFlyFrm::operator<"

sal_Bool SwPosFlyFrm::operator<( const SwPosFlyFrm& rPosFly )
{
    if( pNdIdx->GetIndex() == rPosFly.pNdIdx->GetIndex() )
    {
        // dann entscheidet die Ordnungsnummer!
        return nOrdNum < rPosFly.nOrdNum;
    }
    return pNdIdx->GetIndex() < rPosFly.pNdIdx->GetIndex();
}

Is it enough to only compare "index" in new function
"SwPosFlyFrmCmp::operator()"

bool SwPosFlyFrmCmp::operator()(const SwPosFlyFrmPtr& rA, const SwPosFlyFrmPtr&
rB) const 
{ 
    return rA->GetNdIndex() < rB->GetNdIndex(); 
}

Should it be
bool SwPosFlyFrmCmp::operator()(const SwPosFlyFrmPtr& rA, const SwPosFlyFrmPtr&
rB) const 
{
    if (rA->GetNdIndex() == rB->GetNdIndex())
    {
         return rA->GetOrdNum() < rB->GetOrdNum();
    }

    return rA->GetNdIndex() < rB->GetNdIndex(); 
}

I think that we should keep the existing logic as many as we can in only
replacing the STL container.

Thanks!

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

Mime
View raw message