Re: [Libreoffice] Mi first very little patch

On Sun, Sep 11, 2011 at 2:39 PM, CaStarCo <castarco@gmail.com> wrote:
> Hello, I've created a very little patch to solve the next CppCheck warning (
> http://libreoffice.boldandbusted.com/464.html ) . (This is my first patch in
> LibreOffice)

Welcome to libreoffice and thanks for the patch.

Unfortunately you got the parenthses wrong...

--- a/sw/source/core/crsr/findfmt.cxx
+++ b/sw/source/core/crsr/findfmt.cxx
@@ -61,7 +61,7 @@ sal_Bool SwPaM::Find( const SwFmt& rFmt, SwMoveFn fnMove,
     while( !bFound &&
             0 != ( pNode = ::GetNode( *pPam, bFirst, fnMove, bInReadOnly )))
     {
-        if( 0 != ( bFound = pNode->GetFmtColl() == &rFmt ))
+        if( 0 != ( ( bFound = pNode->GetFmtColl() ) == &rFmt ))

here bFound is a boolean. The intent is to check if pNode->getFmtCol()
is equal to &rFmt. the original author decided to store that test in
boFound (although that is not needed here, because if boFound is true
(ie != 0) then we break out of the while loop and never need to test
boFound again...


so

the minimum patch should have been

+        if( 0 != ( bFound = (pNode->GetFmtColl()  == &rFmt )


a more 'involved' clean-up could have been (considering that th
function already has more than one return point)

diff --git a/sw/source/core/crsr/findfmt.cxx b/sw/source/core/crsr/findfmt.cxx
index 7a548c3..5b29686 100644
--- a/sw/source/core/crsr/findfmt.cxx
+++ b/sw/source/core/crsr/findfmt.cxx
@@ -37,7 +37,6 @@
 sal_Bool SwPaM::Find( const SwFmt& rFmt, SwMoveFn fnMove,
                         const SwPaM *pRegion, sal_Bool bInReadOnly  )
 {
-    sal_Bool bFound = sal_False;
     sal_Bool bSrchForward = fnMove == fnMoveForward;
     SwPaM* pPam = MakeRegion( fnMove, pRegion );

@@ -58,10 +57,9 @@ sal_Bool SwPaM::Find( const SwFmt& rFmt, SwMoveFn fnMove,

     sal_Bool bFirst = sal_True;
     SwCntntNode* pNode;
-    while( !bFound &&
-            0 != ( pNode = ::GetNode( *pPam, bFirst, fnMove, bInReadOnly )))
+    while( (pNode = ::GetNode( *pPam, bFirst, fnMove, bInReadOnly )) != 0)
     {
-        if( 0 != ( bFound = pNode->GetFmtColl() == &rFmt ))
+        if( pNode->GetFmtColl() == &rFmt )
         {
             // wurde die FormatCollection gefunden, dann handelt es sich auf
             // jedenfall um einen SwCntntNode !!
@@ -75,11 +73,13 @@ sal_Bool SwPaM::Find( const SwFmt& rFmt, SwMoveFn fnMove,
             GetMark()->nContent = 0;
             if( !bSrchForward )         // rueckwaerts Suche?
                 Exchange();             // SPoint und GetMark tauschen
+            delete pPam;
+            return sal_True;
             break;
         }
     }
     delete pPam;
-    return bFound;
+    return sal_False;
 }

 Norbert