Re: [PATCH] fix proposed for fdo#48683

Hi Julien,

On Sat, Apr 14, 2012 at 11:08 AM, Kohei Yoshida <kohei.yoshida@gmail.com> wrote:

> I did refactor this part, and now the text portion container manages
> the life cycle of stored containers.  There is no need to delete
> stored elements outside of it.  So, you did the right thing here.

While you are at it, the same method still contains two other delete
calls toward the bottom.  You can remove them also.

One gotcha is that, in the following code block near the bottom of the method

            pParaPortion->GetTextPortions().Remove( nLastPortion );
            if ( nLastPortion && pTP->GetLen() )
            {
                TextPortion* pPrev =
pParaPortion->GetTextPortions()[nLastPortion - 1];
                DBG_ASSERT( pPrev->GetKind() == PORTIONKIND_TEXT, "Portion?!" );
                pPrev->SetLen( pPrev->GetLen() + pTP->GetLen() );
                pPrev->GetSize().Width() = (-1);
            }
            delete pTP;

You need to move the Remove() call to after the if block, since the
object pointed to by pTP is still accessed in the if block.  Leaving
it where it is would cause a segfault.

Kohei