Re: Advice needed about some cppcheck reports

On 28/05/12 18:03, julien2412 wrote:
> Hello,
>
> Here are some cases found by cppcheck and I don't know what to do for them :
> [sw/source/ui/docvw/SidebarWin.cxx:796] ->
> [sw/source/ui/docvw/SidebarWin.cxx:794]: (style) Found duplicate branches
> for if and else.
>
>     791     const SwViewOption* pVOpt =
> mrView.GetWrtShellPtr()->GetViewOptions();
>     792     sal_uLong nCntrl = Engine()->GetControlWord();
>     793     // turn off
>     794     if (!pVOpt->IsOnlineSpell())
>     795         nCntrl &= ~EE_CNTRL_ONLINESPELLING;
>     796     else
>     797         nCntrl &= ~EE_CNTRL_ONLINESPELLING;
>     798     Engine()->SetControlWord(nCntrl);
>     799 
>     800     //turn back on
>     801     if (pVOpt->IsOnlineSpell())
>     802         nCntrl |= EE_CNTRL_ONLINESPELLING;
>     803     else
>     804         nCntrl &= ~EE_CNTRL_ONLINESPELLING;
>     805     Engine()->SetControlWord(nCntrl);
> => Just remove the if because we want to turn off in both cases ?

yes, clearly the intent here is to turn it off always and then perhaps
back on.

> [sw/source/ui/shells/langhelper.cxx:214] ->
> [sw/source/ui/shells/langhelper.cxx:212]: (style) Found duplicate branches
> for if and else.
>     209                     const SwViewOption* pVOpt =
> rView.GetWrtShellPtr()->GetViewOptions();
>     210                     sal_uLong nCntrl =
> pEditEngine->GetControlWord();
>     211                     // turn off
>     212                     if (!pVOpt->IsOnlineSpell())
>     213                         nCntrl &= ~EE_CNTRL_ONLINESPELLING;
>     214                     else
>     215                         nCntrl &= ~EE_CNTRL_ONLINESPELLING;
>     216                     pEditEngine->SetControlWord(nCntrl);
>     217 
>     218                     //turn back on
>     219                     if (pVOpt->IsOnlineSpell())
>     220                         nCntrl |= EE_CNTRL_ONLINESPELLING;
>     221                     else
>     222                         nCntrl &= ~EE_CNTRL_ONLINESPELLING;
>     223                     pEditEngine->SetControlWord(nCntrl);
> => Idem former case ?

yes

> [connectivity/source/drivers/mozab/MDriver.cxx:240] ->
> [connectivity/source/drivers/mozab/MDriver.cxx:238]: (style) Found duplicate
> branches for if and else.
>     238         else if(url ==
> ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("sdbc:address:")) )
>     239             return Unknown; // TODO check
>     240         else
>     241             return Unknown;
> In 2010-11-19 was in the form "else if(url ==
> ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("sdbc:address:")) )" and before,
> like this since 2004-08-02

no idea, presumably TODO indicates there should be special handling for
the sdbc:address case?

> [sw/source/core/unocore/unomap.cxx:965] ->
> [sw/source/core/unocore/unomap.cxx:965]: (style) Same expression on both
> sides of '|'.
> [sw/source/core/unocore/unomap.cxx:968] ->
> [sw/source/core/unocore/unomap.cxx:968]: (style) Same expression on both
> sides of '|'.
> [sw/source/core/unocore/unomap.cxx:969] ->
> [sw/source/core/unocore/unomap.cxx:969]: (style) Same expression on both
> sides of '|'.
>     965                     { SW_PROP_NMID(UNO_NAME_BACK_COLOR),
> FN_UNO_TABLE_CELL_BACKGROUND,  CPPU_E2T(CPPUTYPE_INT32),  
> PropertyAttribute::MAYBEVOID|PropertyAttribute::MAYBEVOID ,MID_BACK_COLOR      
> },
>     966                     { SW_PROP_NMID(UNO_NAME_BACK_GRAPHIC_URL),
> RES_BACKGROUND,      CPPU_E2T(CPPUTYPE_OUSTRING),
> PropertyAttribute::MAYBEVOID ,MID_GRAPHIC_URL    },
>     967                     { SW_PROP_NMID(UNO_NAME_BACK_GRAPHIC_FILTER),
> RES_BACKGROUND,       CPPU_E2T(CPPUTYPE_OUSTRING),
> PropertyAttribute::MAYBEVOID ,MID_GRAPHIC_FILTER    },
>     968                     { SW_PROP_NMID(UNO_NAME_BACK_GRAPHIC_LOCATION),
> FN_UNO_TABLE_CELL_BACKGROUND,   CPPU_E2T(CPPUTYPE_GRAPHICLOC),
> PropertyAttribute::MAYBEVOID|PropertyAttribute::MAYBEVOID
> ,MID_GRAPHIC_POSITION},
>     969                     { SW_PROP_NMID(UNO_NAME_BACK_TRANSPARENT),
> FN_UNO_TABLE_CELL_BACKGROUND,    CPPU_E2T(CPPUTYPE_BOOLEAN),
> PropertyAttribute::MAYBEVOID|PropertyAttribute::MAYBEVOID
> ,MID_GRAPHIC_TRANSPARENT      },
> => just remove extra PropertyAttribute::MAYBEVOID ?

it looks to me like none of the other PropertyAttribute values make
sense here, so just remove the duplicate.

> [sal/osl/unx/file.cxx:1261] -> [sal/osl/unx/file.cxx:1261]: (style) Same
> expression on both sides of '-'.
>    1257             if (nSize > 0)
>    1258             {
>    1259                 c^= pData[0];
>    1260                 pData += nSize;
>    1261                 nSize -= nSize;
>    1262             }
> Just put nSize to 0 ?

makes sense

> Same thing here :
> [sal/osl/w32/file.cxx:880] -> [sal/osl/w32/file.cxx:880]: (style) Same
> expression on both sides of '-'.
>     876         if (nSize > 0)
>     877         {
>     878             c ^= pData[0];
>     879             pData += nSize;
>     880             nSize -= nSize;
>     881         }
>
> And here :
> [filter/source/graphicfilter/icgm/cgm.cxx:269] ->
> [filter/source/graphicfilter/icgm/cgm.cxx:269]: (style) Same expression on
> both sides of '-'.
>     267             if ( pLong[ nSwitch ] < 0 )
>     268             {
>     269                 nRetValue -= nRetValue;
>     270             }
>     271             nRetValue /= 65536;

also makes sense