About coverity 705746 (pormulti.cxx from sw module)

[Julien]

De : Noel Grandin <noel at peralex.com>
À : julien2412 <serval2412 at yahoo.fr> 
Cc : libreoffice at lists.freedesktop.org 
Envoyé le : Vendredi 15 mars 2013 10h04
Objet : Re: About coverity 705746 (pormulti.cxx from sw module)
 

On 2013-03-15 10:30, julien2412 wrote:

Noel Grandin wrote 
>I would guess that someone meant to move line 742 inside the first "if" 
statement, but only did half the job. 
>Hello Noël, Perhaps I'm too sleepy this morning but I don't see the interest to
duplicate the "new" part. Any hint? 
I think the code should read:

 if( nLeft || nRight ) 
     { 
         if( !pCurr->GetPortion() ) 
             pCurr->SetPortion( new SwTxtPortion( *pCurr ) ); 
         if( nLeft ) 
         { 
             SwMarginPortion *pMarg = new SwMarginPortion( 0 ); 
             pMarg->AddPrtWidth( nLeft ); 
             pMarg->SetPortion( pCurr->GetPortion() ); 
             pCurr->SetPortion( pMarg ); 
         } 
         if( nRight ) 
         { 
            SwMarginPortion *pMarg = new SwMarginPortion( 0 ); 
             pMarg->AddPrtWidth( nRight ); 
             pCurr->FindLastPortion()->Append( pMarg ); 
         } 
    }

________________________________
Disclaimer: http://www.peralex.com/disclaimer.html

Since we have "if( nLeft || nRight )", what about:
 if( nLeft || nRight ) 
     { 
         if( !pCurr->GetPortion() ) 
             pCurr->SetPortion( new SwTxtPortion( *pCurr ) );
       SwMarginPortion *pMarg = new SwMarginPortion( 0 );
         if( nLeft ) 
         { 
             pMarg->AddPrtWidth( nLeft ); 
             pMarg->SetPortion( pCurr->GetPortion() ); 
             pCurr->SetPortion( pMarg ); 
         } 
         if( nRight )
         { 
             pMarg->AddPrtWidth( nRight ); 
             pCurr->FindLastPortion()->Append( pMarg ); 
         } 
    }

If nLeft and nRight are exlusive, it could also just be an if(nLeft)... else...

In both cases, no need to repeat "SwMarginPortion *pMarg = new SwMarginPortion( 0 );" since we're sure to be in one of these blocks.

Julien
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20130315/3de14d14/attachment-0001.html>